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

🐛Signed-Releases: really look for *.sign files #1298

Merged
merged 3 commits into from
Nov 20, 2021
Merged

🐛Signed-Releases: really look for *.sign files #1298

merged 3 commits into from
Nov 20, 2021

Conversation

evverx
Copy link
Contributor

@evverx evverx commented Nov 18, 2021

No description provided.

With this patch applied projects like dracut pass the check:
```
  "checks": [
    {
      "details": [
        "Debug: GitHub release found: 055",
        "Info: signed release artifact: dracut-055.tar.sign: https://api.github.com/repos/dracutdevs/dracut/releases/assets/37635937",
        "Debug: GitHub release found: 054",
        "Info: signed release artifact: dracut-054.tar.sign: https://api.github.com/repos/dracutdevs/dracut/releases/assets/36958052",
        "Debug: GitHub release found: 053",
        "Info: signed release artifact: dracut-053.tar.sign: https://api.github.com/repos/dracutdevs/dracut/releases/assets/32484038",
        "Debug: GitHub release found: 052",
        "Info: signed release artifact: dracut-052.tar.sign: https://api.github.com/repos/dracutdevs/dracut/releases/assets/32130796",
        "Debug: GitHub release found: 051",
        "Info: signed release artifact: dracut-051.tar.sign: https://api.github.com/repos/dracutdevs/dracut/releases/assets/31933850"
      ],
      "score": 10,
      "reason": "5 out of 5 artifacts are signed -- score normalized to 10",
      "name": "Signed-Releases",
```
Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

Thanks!

@laurentsimon laurentsimon enabled auto-merge (squash) November 18, 2021 16:41
auto-merge was automatically disabled November 19, 2021 00:52

Head branch was pushed to by a user without write access

@evverx
Copy link
Contributor Author

evverx commented Nov 19, 2021

@laurentsimon the PR was out of date with the main branch so I rebased and pushed it once again disabling "auto-merge" along the way. I also updated the commit message to make it pass the "description" check. Could you take a look?

@laurentsimon laurentsimon enabled auto-merge (squash) November 19, 2021 01:17
@laurentsimon
Copy link
Contributor

the integration tests are failing, we need to fix it upstream so the PR is blocked atm.

@evverx
Copy link
Contributor Author

evverx commented Nov 19, 2021

Just out of curiosity, is that "out-of-date" check I see every time the scorecard repository gets updated turned on via the "strict" status check mentioned at https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/defining-the-mergeability-of-pull-requests/about-protected-branches#require-status-checks-before-merging? It seems to be too annoying at least in projects receiving a lot of PRs so I'm not sure scorecard should penilize them for not using it. It's totally understandable that maintainers wouldn't want to discourage drive-by contributors

@evverx
Copy link
Contributor Author

evverx commented Nov 19, 2021

I mean, it's probably OK to enforce it when all contributors work at the same company for example and have to follow this policy but it's just strange to enforce it on external contributors.

@evverx
Copy link
Contributor Author

evverx commented Nov 19, 2021

Just to clarify, I've opened 3 PRs and as far as I understand basically I'll have to rebase and push all of them one way or another once the integration test is fixed. Personally I think it's a waste of time given there are no any merge conflicts there and I certainly wouldn't want to waste other people's time with this strict check unless I was ready to rebase PRs like that myself (which I'm definitely not :-))

@naveensrinivasan naveensrinivasan temporarily deployed to integration-test November 20, 2021 00:09 Inactive
@github-actions
Copy link

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

@naveensrinivasan naveensrinivasan temporarily deployed to integration-test November 20, 2021 00:24 Inactive
@github-actions
Copy link

Integration tests success for
[786dbc7]
(https://github.com/ossf/scorecard/actions/runs/1483263247)

@laurentsimon laurentsimon merged commit 9d29765 into ossf:main Nov 20, 2021
@@ -28,7 +28,7 @@ const (
releaseLookBack = 5
)

var artifactExtensions = []string{".asc", ".minisig", ".sig"}

Choose a reason for hiding this comment

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

Var

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.

4 participants