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

feat(cve): update columns #184

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

kahboom
Copy link
Contributor

@kahboom kahboom commented Oct 7, 2024

relates to #171

Screenshot 2024-10-07 at 3 28 13 PM

Still need to add the Published column and check the CVSS column.

Copy link
Member

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

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

  • As described in the spreadsheet I think the "Advisories" Page will be removed. So rather than fixing issues there, we might take the opportunity and delete the directories:
    • /client/src/app/pages/advisory-details
    • /client/src/app/pages/advisory
      And also remove the Advisory from the Routes + remove it from the sidebar. What do you think? Or is it too premature to do it? I'll leave that up to you.

I'll add more comments to the other lines in a moment :)

@carlosthe19916
Copy link
Member

Also, the IDE is supposed to help guiding us to select the appropriate value for each field so we don't commit spelling mistakes, or wrong choices.

image

Could you confirm the IDE does not help guiding you please? Perhaps there might be something missing if that is not the case

@kahboom
Copy link
Contributor Author

kahboom commented Oct 7, 2024

@carlosthe19916 thanks for your review, I've made the changes it builds fine now. Looking at a few things I'm missing like Published, will update shortly.

we might take the opportunity and delete the directories
And also remove the Advisory from the Routes + remove it from the sidebar. What do you think? Or is it too premature to do it? I'll leave that up to you.

Given that we are still making a lot of changes to the design are we 100% sure we won't want to reuse that code? I don't mind either way.

Could you confirm the IDE does not help guiding you please?

Yes, no issues with autocomplete though it's not 100% happy. Not blocking me though so I'm not worrying about it for now.

fix: column keys

add published column
@carlosthe19916
Copy link
Member

Given that we are still making a lot of changes to the design are we 100% sure we won't want to reuse that code? I don't mind either way.

@kahboom I had a chat with our PM @mid998 and we agreed to remove the "Advisory" from the left sidebar. But let's do it in a separate PR as it will be easier to track it if at any point we need to revisit the deleted code

@kahboom kahboom marked this pull request as ready for review October 7, 2024 14:54
Copy link
Member

@carlosthe19916 carlosthe19916 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @kahboom

@carlosthe19916 carlosthe19916 merged commit 9bfdb5e into trustification:main Oct 7, 2024
7 checks passed
@kahboom kahboom deleted the 171-cve-list branch October 7, 2024 14:57
@kahboom kahboom mentioned this pull request Oct 7, 2024
5 tasks
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.

2 participants