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

Added bold text to package and color to CVE only. #38

Merged
merged 0 commits into from
Feb 28, 2020
Merged

Added bold text to package and color to CVE only. #38

merged 0 commits into from
Feb 28, 2020

Conversation

joshuachp
Copy link
Contributor

Clean up and reverted to first commit.

Formatted the output like Package {pkg} is affected by {severity} {issues}. with the pkg in bold and the severity and issue colored.

@joshuachp joshuachp changed the title Pull Request: Added bold text to package and color to CVE only. Added bold text to package and color to CVE only. Feb 14, 2020
@ilpianista
Copy link
Owner

ilpianista commented Feb 14, 2020

There was no need to close the previous PR #36.

This has the same problem I wrote in #36 (comment)

@joshuachp
Copy link
Contributor Author

I created a new branch to integrate the other merge and start fresh.

I like the bold text, but I don't like to have the CVEs listed after the risk score. Mainly because you can have multiple CVEs affecting one package and the risk score isn't related to all of them, but to the riskier.

If there is only one CVE the color should be correct. If there is more than one I think the highest color is more relevant. It is also the current implementation. I just wanted to make a simple change from the full colored line. If it doesn't fit it doesn't matter.

@ilpianista
Copy link
Owner

Then, please don't color the CVEs at all and put the risk back to the end.

@joshuachp
Copy link
Contributor Author

Pushed the changes.

src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
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