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(tooling): fix all report links and defers all audit content to the new audit section #1233

Merged
merged 2 commits into from
Oct 21, 2020

Conversation

hugomrdias
Copy link
Contributor

@hugomrdias hugomrdias commented Oct 21, 2020

This PR fixes all the theory and security links that this PR #1230 broke.

Also now that we have a new audit section with the all the audits content, we can just link to a specific sub section.

Screenshot 2020-10-21 at 12 17 47

Screenshot 2020-10-21 at 12 16 33

Some frontmatter properties were removed because the audit section already has them and more.

closes #1232

Copy link
Collaborator

@yiannisbot yiannisbot left a comment

Choose a reason for hiding this comment

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

Looks good! See a comment regarding the latest audit report date in the dashboard. Plus, can we please underline the dashboard entry (currently "Reports", or the actual date according to the comment), so that it's more clear that it's a link? Right now, you have hover over it in order to see it's a link.

@@ -5,7 +5,7 @@ dashboardWeight: 2.0
dashboardState: stable
dashboardTests: 0
dashboardAudit: done
dashboardAuditURL: /#section-appendix.audit_reports.2020-06-03-gossipsub-design-and-implementation
dashboardAuditURL: /#section-appendix.audit_reports.gossipsub
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you excluding the date from the section header so that it always points to this subsection and then multiple audits can be listed within the subsection without having to change the dashboardAuditURL every time someone adds a new report for the same protocol/repo? That's a good idea I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly

@@ -5,7 +5,7 @@ dashboardWeight: 2.0
dashboardState: stable
dashboardTests: 0
dashboardAudit: done
dashboardAuditURL: /#section-appendix.audit_reports.2020-06-03-gossipsub-design-and-implementation
dashboardAuditURL: /#section-appendix.audit_reports.gossipsub
dashboardAuditDate: '2020-06-03'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to have the date of the latest report shown in the dashboard. Can it be pulled from here (dashboardAuditDate)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the problem is that it will be out of date with the audit section quickly, we either use the audit section content as the source of truth or the frontmatter properties.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see the point, but I think it's a requirement to have the date of the latest audit in the dashboard. I know this is in contrast to the change just above.

@daviddias are we ok if we don't have the latest audit date in the dashboard?

@hugomrdias
Copy link
Contributor Author

hugomrdias commented Oct 21, 2020

Screenshot 2020-10-21 at 15 25 45

@hugomrdias hugomrdias requested a review from yiannisbot October 21, 2020 14:26
@hugomrdias hugomrdias merged commit 1b7a161 into master Oct 21, 2020
@hugomrdias hugomrdias deleted the feat/reports branch October 21, 2020 15:41
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.

Inconsistency of audit reports in spec dashboards
2 participants