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.

Sign up for GitHub

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

Jump to bottom

📖 Security-Policy: Link to responsible disclosure guidelines for remediation #1545

Merged
merged 5 commits into from
Jan 27, 2022
Merged

📖 Security-Policy: Link to responsible disclosure guidelines for remediation #1545

merged 5 commits into from
Jan 27, 2022

Conversation

laurentsimon
Copy link
Contributor

docs/checks.md: updated the doc
docs/checks/internal/checks.yaml: updated the source of truth for docs

Link to the responsible disclosure guidelines.

No breaking changes.

@laurentsimon laurentsimon requested a review from olivekl as a code owner January 27, 2022 17:19
@laurentsimon laurentsimon temporarily deployed to integration-test January 27, 2022 17:19 Inactive
@laurentsimon laurentsimon enabled auto-merge (squash) January 27, 2022 17:20
@laurentsimon laurentsimon temporarily deployed to integration-test January 27, 2022 17:20 Inactive
@laurentsimon laurentsimon temporarily deployed to integration-test January 27, 2022 17:21 Inactive
@github-actions
Copy link

Integration tests success for
[0096c83]
(https://github.com/ossf/scorecard/actions/runs/1757537190)

@github-actions
Copy link

Integration tests success for
[f3fbaa3]
(https://github.com/ossf/scorecard/actions/runs/1757541648)

@github-actions
Copy link

Integration tests success for
[16d236d]
(https://github.com/ossf/scorecard/actions/runs/1757544104)

justaugustus
justaugustus requested changes Jan 27, 2022
View reviewed changes
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

@laurentsimon -- Thanks for adding this!

Microscopic nits:

  • e.g., needs a comma and shouldn't be parenthetical
  • extraneous space

Added a code suggestion for both!

docs/checks.md Outdated Show resolved Hide resolved
justaugustus
justaugustus reviewed Jan 27, 2022
View reviewed changes
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

@laurentsimon -- Another nit to briefly continue the convo from #1532 (comment):

The notes you left in the PR description are much clearer!

docs/checks.md: updated the doc
docs/checks/internal/checks.yaml: updated the source of truth for docs

What I was suggesting in the previous PR was to make these not the PR description, but the actual commit messages.

That way, when the PR content gets squashed and merged, the details get included as part of the git history.

@justaugustus justaugustus changed the title Link to responsible disclosure guidelines in Security-Policy remediation doc 📖 Link to responsible disclosure guidelines in Security-Policy remediation doc Jan 27, 2022
@justaugustus justaugustus changed the title 📖 Link to responsible disclosure guidelines in Security-Policy remediation doc 📖 Security-Policy: Link to responsible disclosure guidelines for remediation Jan 27, 2022
@laurentsimon
Copy link
Contributor Author

@laurentsimon -- Another nit to briefly continue the convo from #1532 (comment):

The notes you left in the PR description are much clearer!

docs/checks.md: updated the doc
docs/checks/internal/checks.yaml: updated the source of truth for docs

What I was suggesting in the previous PR was to make these not the PR description, but the actual commit messages.

That way, when the PR content gets squashed and merged, the details get included as part of the git history.

Gocha. The info also seems useful for the PR reviewer. So need them in both places?

@laurentsimon laurentsimon temporarily deployed to integration-test January 27, 2022 18:26 Inactive
@laurentsimon
Copy link
Contributor Author

@laurentsimon -- Another nit to briefly continue the convo from #1532 (comment):
The notes you left in the PR description are much clearer!

docs/checks.md: updated the doc
docs/checks/internal/checks.yaml: updated the source of truth for docs

What I was suggesting in the previous PR was to make these not the PR description, but the actual commit messages.
That way, when the PR content gets squashed and merged, the details get included as part of the git history.

Gocha. The info also seems useful for the PR reviewer. So need them in both places?

do you have a doc on updating a past commit? I can try adding it

@github-actions
Copy link

Integration tests success for
[d8ca4bc]
(https://github.com/ossf/scorecard/actions/runs/1757822404)

@github-actions
Copy link

Integration tests success for
[ff9d305]
(https://github.com/ossf/scorecard/actions/runs/1757821651)

olivekl
olivekl approved these changes Jan 27, 2022
View reviewed changes
Copy link
Contributor

@olivekl olivekl left a comment

Choose a reason for hiding this comment

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

LGTM!

docs/checks.md Outdated Show resolved Hide resolved
@laurentsimon laurentsimon temporarily deployed to integration-test January 27, 2022 19:41 Inactive
@github-actions
Copy link

Integration tests success for
[2101616]
(https://github.com/ossf/scorecard/actions/runs/1758128860)

@codecov-commenter
Copy link

Codecov Report

Merging #1545 (2101616) into main (17467c1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1545   +/-   ##
=======================================
  Coverage   52.76%   52.76%           
=======================================
  Files          70       70           
  Lines        6178     6178           
=======================================
  Hits         3260     3260           
  Misses       2712     2712           
  Partials      206      206

@justaugustus justaugustus mentioned this pull request Jan 27, 2022
Closed
justaugustus
justaugustus approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

do you have a doc on updating a past commit? I can try adding it

@laurentsimon -- Opened a general tracking issue for contribution doc improvements: #1553

No need to worry about it here; I can tweak the merge message!

@laurentsimon laurentsimon merged commit 40a9d48 into ossf:main Jan 27, 2022
@justaugustus
Copy link
Member

I can tweak the merge message!

nvm, auto-merge was enabled 🙃

@laurentsimon
Copy link
Contributor Author

I can tweak the merge message!

nvm, auto-merge was enabled 🙃

oops sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olivekl olivekl olivekl approved these changes

@justaugustus justaugustus justaugustus approved these changes

@naveensrinivasan naveensrinivasan Awaiting requested review from naveensrinivasan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@laurentsimon @codecov-commenter @justaugustus @olivekl