-
Notifications
You must be signed in to change notification settings - Fork 20
feat: separate Red Hat and Community profiles for HTML report #522
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
Conversation
cdf0663 to
2478217
Compare
2478217 to
0d9d642
Compare
ruromero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the right approach but let's make it more generic and move the brandingConfig to the ReportTemplate.java
507681f to
cc10296
Compare
|
@ruromero I have updated according to your comments and added the back-end general solution too. Could you review again ? thanks. |
cc10296 to
07d3764
Compare
ruromero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't added a comment in each place where Red Hat is mentioned but it should be deleted everywhere.
The use of the icon should allow any arbitrary icon and we will override it in the private project that we will deploy in the cloud.
Also don't repeat the default values each time.
src/main/java/io/github/guacsec/trustifyda/integration/report/ReportTemplate.java
Outdated
Show resolved
Hide resolved
07d3764 to
638a314
Compare
638a314 to
bfaf296
Compare
ruromero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the changes. Some small remarks and suggestions.
src/main/java/io/github/guacsec/trustifyda/integration/report/ReportTemplate.java
Outdated
Show resolved
Hide resolved
src/main/java/io/github/guacsec/trustifyda/integration/report/ReportTemplate.java
Outdated
Show resolved
Hide resolved
c9ea1f1 to
f55ff1b
Compare
|
I misunderstood the requirement. Now I
Please review again. Thanks. |
712c43e to
fc4470b
Compare
…ngConfig and update README
fc4470b to
7e953a5
Compare
ruromero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized about the imageRecommendationLink.
Also please make sure you validate all the cases and properties in your local development before merging.
- Icon and properties customization, overrides
- Image text and links
src/test/java/io/github/guacsec/trustifyda/integration/HtmlReportTest.java
Show resolved
Hide resolved
fix: add imageRemediationLink fix: adapt HtmlReportTest by adding imageRecommend and imageRemediationLink to test configuration
ruromero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all the points are now addressed. Thank you
|
Thank you for patiently reviewing. |
feat: separate Red Hat and Community profiles for HTML report
fixes: #523
The community version HTML report is like:
Hi @ruromero, I am making this draft PR to review, of course we might choose other approach.