Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dep Card: Decision Summary #787

Merged
merged 25 commits into from
Jun 13, 2024
Merged

Dep Card: Decision Summary #787

merged 25 commits into from
Jun 13, 2024

Conversation

aclark02-arcus
Copy link
Collaborator

@aclark02-arcus aclark02-arcus commented Jun 4, 2024

Part II of a two part PR to close #774. It also updates that layout of the package dependencies tab (and reports) as well.

Some views of the new card:

  • When no high risk exists:

    image

  • When there is any high risk package, the text and icon turn red:

    image

  • When no decisions exist (or no dependencies)

    image

@aclark02-arcus aclark02-arcus changed the title Add new Dep Card Dep Card: Decision Summary Jun 4, 2024
Base automatically changed from ac-774 to dev June 4, 2024 20:49
@aclark02-arcus aclark02-arcus marked this pull request as ready for review June 5, 2024 16:29
Copy link
Collaborator

@jthompson-arcus jthompson-arcus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few things:

  1. I noticed that the red color when a dependency has a high decision doesn't show up on the HTML report. Is this intentional or an oversight?
  2. In the PDF report I didn't get the line break for the new card.
Snag_10141cda
  1. Getting this warning in the console when "Include Suggests" is selected
Snag_10157c99

R/mod_downloadHandler.R Outdated Show resolved Hide resolved
@aclark02-arcus aclark02-arcus requested review from jthompson-arcus and removed request for jthompson-arcus June 10, 2024 15:40
@jthompson-arcus
Copy link
Collaborator

@aclark02-arcus I found this issue on the dependency tab:
image

Do you want to fix this here or do you want me to open a new issue for it after your PR is merged? Everything else is looking good.

@aclark02-arcus
Copy link
Collaborator Author

do you want me to open a new issue for it after your PR is merged

@jthompson-arcus, that works!

Copy link
Collaborator

@jthompson-arcus jthompson-arcus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the NEWS?

Copy link
Collaborator

@jthompson-arcus jthompson-arcus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jthompson-arcus jthompson-arcus merged commit c250e68 into dev Jun 13, 2024
1 check passed
@jthompson-arcus jthompson-arcus deleted the ac-774-p2 branch June 13, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants