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(content): add implementations audits as a dedicated section in the spec #1214

Merged
merged 7 commits into from
Oct 15, 2020
Merged

Conversation

dkkapur
Copy link
Contributor

@dkkapur dkkapur commented Oct 15, 2020

created an audits section in the appendix where we can collect the links to published audit reports and share them with short descriptions

yiannisbot
yiannisbot previously approved these changes Oct 15, 2020
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.

If the site builds without errors and the links work, then this looks good to go! (CI still pending)

@@ -19,9 +19,9 @@ implRepos:
auditState: done
audits:
- auditDate: '2020-07-28'
auditURL: https://github.com/filecoin-project/rust-fil-proofs/blob/master/audits/protocolai-audit-20200728.pdf
auditURL: /#appendix__audit_reports__rust-fil-proofs__filecoin-proving-subsystem
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you paste a screenshot of what the dashboard looks like?

Copy link
Contributor

Choose a reason for hiding this comment

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

you have the preview below

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

just made a few more changes based on @daviddias comments.

@yiannisbot - here is what the dashboard looks like now:

image

image

@hugomrdias
Copy link
Contributor

can you please rebase onto master?

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

@hugomrdias i think this one should be correctly rebased, please let me know if thats not true

@hugomrdias
Copy link
Contributor

you merged instead of rebase now this PR has a bunch of changed files not related to these changes

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

ah yes sorry - my 4am brain is being bad. should i scrap this PR, reset the head and reopen one?

@hugomrdias
Copy link
Contributor

hugomrdias commented Oct 15, 2020

Basically the urls were wrong in yours changes. Please check now and copy paste the correct relative urls to the dashboard md file.
Use the preview below to check the relative urls.

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

@hugomrdias does it look right now?

@hugomrdias
Copy link
Contributor

yep the link goes to the right section but opens in a new tab.
Are we going to use the audit section for all audit related to implementations ?

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

@hugomrdias yes, that is the plan right now. will also add theory audits to this section in the next day (drand and gossipsub, both of which are linked in the main dashboard right now, and will follow the same process as here with the URLs).

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

what should I do to the URL to not open in a new tab?

@hugomrdias
Copy link
Contributor

Just please make an issue about audit relative urls opening in a new tab and i will fix it. Mention this PR once that issue is up i will merge this into master.

Thank you great job.

@dkkapur
Copy link
Contributor Author

dkkapur commented Oct 15, 2020

thank you! issue is up and this PR is mentioned.

@hugomrdias hugomrdias changed the title adding audits as a dedicated section in the spec feat(content): add implementations audits as a dedicated section in the spec Oct 15, 2020
@hugomrdias hugomrdias merged commit ee2535b into filecoin-project:master Oct 15, 2020
@dkkapur dkkapur deleted the security-audits branch October 15, 2020 11:36
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