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

Show negative results in details #95

Closed
moorereason opened this issue Dec 19, 2020 · 5 comments
Closed

Show negative results in details #95

moorereason opened this issue Dec 19, 2020 · 5 comments
Labels
help wanted Community contributions welcome, maintainers supportive of idea but not a high priority kind/enhancement New feature or request

Comments

@moorereason
Copy link
Contributor

Howdy! New here. 👋 🤠

While using scorecard to evaluate projects I contribute to, I found myself having to read (and patch) the checks code to understand why I was seeing failing or marginal results. It would be very helpful to see negative results in the --show-details output instead of only positive results. It makes it easier for newcomers like me to learn what is considered bad practice, and I assume it would be help everyone to investigate any negative findings.

For example, I've patched scorecard locally to give negative results (starting with !!) for some checks:

$ ./scorecard --repo=github.com/gohugoio/hugo --show-details --checks=Code-Review,CI-Tests,Pull-Requests,Signed-Releases,Signed-Tags
RESULTS
-------
CI-Tests: Fail 4
    !! found committed PR without CI test: 8075
    !! found committed PR without CI test: 8070
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/1056701da088d5e87e6e31cdc6e0c455862697cc
    !! found committed PR without CI test: 8059
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/34ecd28779d4836c74d4e71a5227b196d5d2cbec
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/713792077b06504e9fdd0c8abdc2aebace5dcf2a
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/3a38df2c4f96e560ddedc5fd5266fb54972a7fb6
    !! found committed PR without CI test: 8020
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/7475bb324f208c8acd5fb3b6aa0f1b07c8d26d94
    CI test found: context: continuous-integration/travis-ci/push, url: https://api.github.com/repos/gohugoio/hugo/statuses/cd5a53bb240ef23977c03d2a47cd76f8ee915899
    !! found committed PR without CI test: 7999
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/15ee0346fc60265c36530dcc3b70be3955f006ef
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/bb228c9faf6a44fe2bb60d1d3d6f5ec538a7c786
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/f2b60ecc084b1142c9cf73f7e3736a754067943f
    CI test found: context: continuous-integration/travis-ci/pr, url: https://api.github.com/repos/gohugoio/hugo/statuses/3a89789e40cc093f36cb9c03814f8dc30a9b1921
Code-Review: Pass 8
    found PR with committer different than author: 8075
    found PR with committer different than author: 8070
    !! found unreviewed PR committed by author: 8065
    !! found unreviewed PR committed by author: 8059
    found review approved PR: 8042
    found PR with committer different than author: 8035
    found PR with committer different than author: 8034
    found PR with committer different than author: 8020
    found review approved PR: 8008
    found PR with committer different than author: 8004
    found PR with committer different than author: 7999
    found PR with committer different than author: 7998
    !! found unreviewed PR committed by author: 7991
    found PR with committer different than author: 7989
    found PR with committer different than author: 7988
    github code reviews found for 12 of 15 merged PRs
Pull-Requests: Fail 5
    !! found commit without PR: 6c294182788f7da358243c4a0ef0a98772491067
    !! found commit without PR: 10ae7c3210cd1add14d3750aa9512a87df0e1146
    !! found commit without PR: a2d146ec32a26ccca9ffa68d3c840ec5b08cca96
    !! found commit without PR: 21fa1e86f2aa929fb0983a0cc3dc4e271ea1cc54
    !! found commit without PR: c84ad8db821c10225c0e603c6ec920c67b6ce36f
    !! found commit without PR: 718e09ed4bc538f4fccc4337f99e9eb86aea31f3
    !! found commit without PR: 32d4bf68da7d16302f138dde343c70f9667933c4
    !! found commit without PR: 1415efdcd838cf482072ef08e765a8ce960bfdde
    !! found commit without PR: 4e6bf7907dc5dfde697ace251b91a9399e0e3c39
    !! found commit without PR: 50be4370b0c46c6c34430eb45bdc53d1926dd800
    !! found commit without PR: 3d2e6a30d43079d48eb241505d5e0d9628dedf15
    !! found commit without PR: 4f1e4bb3fe8241d7a900f57e156f9679768aff24
    !! found commit without PR: 9f1265fde4b9ef186148337c99f08601633b6056
    !! found commit without PR: d162bbd7990b6a523bdadcd10bf60fcb43ecf270
    found PRs for 16 out of 30 commits
Signed-Releases: Fail 10
    release found: v0.79.1
    !! release v0.79.1 has no signed artifacts
    release found: v0.79.0
    !! release v0.79.0 has no signed artifacts
    release found: v0.78.2
    !! release v0.78.2 has no signed artifacts
    release found: v0.78.1
    !! release v0.78.1 has no signed artifacts
    release found: v0.78.0
    !! release v0.78.0 has no signed artifacts
    release found: v0.77.0
    !! release v0.77.0 has no signed artifacts
Signed-Tags: Fail 10
    !! unsigned tag found: v0.78.0, commit: 2f1a31211c08f7fd52738ba2f817055a7fab9373
    !! unsigned tag found: v0.78.1, commit: 0cb2fd5cc8eca7cd6af59e246492f1587b69819b
    !! unsigned tag found: v0.78.2, commit: 21103fa0df2b9d23e5792ac4f27def0453118f7e
    !! unsigned tag found: v0.79.0, commit: 626facbfa32823bc2d1152f97e6db67ed051a307
    !! unsigned tag found: v0.79.1, commit: ea1d515f9750581769311d95064f52165c89edd0

I can submit a PR for the above if this idea is acceptable.
@inferno-chromium
Copy link
Contributor

Love the idea, can you submit a PR for all checks!

@moorereason moorereason mentioned this issue Dec 20, 2020
Closed
moorereason added a commit to moorereason/scorecard that referenced this issue Dec 20, 2020
@moorereason
Show negative results to Pull-Requests details
4ec34e9 
Negative results logged with a "!!" prefix.

Updates ossf#95

$ go run . --repo=github.com/ossf/scorecard --show-details --checks=Pull-Requests
Starting [Pull-Requests]
Finished [Pull-Requests]

RESULTS
-------
Pull-Requests: Pass 9
    !! found commit without PR: 71dace5, committer: dlorenc
    found PRs for 29 out of 30 commits
moorereason added a commit to moorereason/scorecard that referenced this issue Dec 20, 2020
@moorereason
Show negative results in Signed-Tags details
eb0d488 
Negative results logged with a "!!" prefix.

Updates ossf#95

$ go run . --repo=github.com/cilium/cilium --show-details --checks=Signed-Tags
Starting [Signed-Tags]
Finished [Signed-Tags]

RESULTS
-------
Signed-Tags: Fail 4
    verified tag found: v1.9.0-rc1, commit: a46b5c308779b00676bcbffe6847701984fb7ec7
    !! unverified tag found: v1.9.0-rc2, commit: 2ee8e4659ad4050154eb83008ba6434bddad44eb, reason: unsigned
    verified tag found: v1.9.0-rc3, commit: ee77e846a9b85e318d6d077c801e2615d5e7dbe3
    !! unverified tag found: v1.9.0, commit: 1cdd547dce26adb046d117494d559c64007365fd, reason: unsigned
    verified tag found: v1.9.1, commit: bb4abe1720cb56c6a5f74d0567665555ad8434f1
    found 3 of 5 verified tags
moorereason added a commit to moorereason/scorecard that referenced this issue Dec 20, 2020
@moorereason
Show negative results in Signed-Releases details
348bedb 
Negative results logged with a "!!" prefix.

Updates ossf#95

$ go run . --repo=github.com/gohugoio/hugo --show-details --checks=Signed-Releases
Starting [Signed-Releases]
Finished [Signed-Releases]

RESULTS
-------
Signed-Releases: Fail 10
    release found: v0.79.1
    !! release v0.79.1 has no signed artifacts
    release found: v0.79.0
    !! release v0.79.0 has no signed artifacts
    release found: v0.78.2
    !! release v0.78.2 has no signed artifacts
    release found: v0.78.1
    !! release v0.78.1 has no signed artifacts
    release found: v0.78.0
    !! release v0.78.0 has no signed artifacts
    release found: v0.77.0
    !! release v0.77.0 has no signed artifacts
    found signed artifacts for 0 of 6 releases
@moorereason moorereason mentioned this issue Dec 20, 2020
Merged
moorereason added a commit to moorereason/scorecard that referenced this issue Dec 23, 2020
@moorereason
Refactor CI-Tests to show negative results
39464a5 
Add negative check results to the CI-Tests output.

Assuming that a repo will only support one CI system, GithubStatuses and
GithubCheckRuns are merged into a single CITests function.  Since both
GithubStatuses and GithubCheckRuns were essentially validating the same
PRs, it makes more sense to keep all of that state together in a single
check.

Additionaly, a single check can reduce the number of API queries once we
detect the CI system in use.

Fixes ossf#96
Updates ossf#95
@moorereason moorereason mentioned this issue Dec 23, 2020
Merged
@moorereason
Copy link
Contributor Author

@inferno-chromium,
The only check I was planning to update is Code-Review, but it will require refactoring the multichecks to avoid lots of false-negative logging while trying to determine what kind of review workflow is in use (similar to #108).

@naveensrinivasan naveensrinivasan added the help wanted Community contributions welcome, maintainers supportive of idea but not a high priority label Feb 16, 2021
@inferno-chromium inferno-chromium added the kind/enhancement New feature or request label Feb 27, 2021
@laurentsimon
Copy link
Contributor

Can you try running the current version of scorecard? We've tried to generally address this.
Looking forward to your feedback!

@evverx evverx mentioned this issue Feb 14, 2022
Closed
@afmarcum
Copy link
Contributor

Is this something that still needs to be discussed considering the user submitting the issue did not provide feedback on the updated version?
If there is no feedback in the next 7 days on whether this remains important for the project, then this issue will be closed.

@spencerschrock
Copy link
Contributor

This issue is already completed. All score deductions will have a WARN associated them in the detail logger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Community contributions welcome, maintainers supportive of idea but not a high priority kind/enhancement New feature or request
Projects
Scorecard
Status: Done
Milestone
No milestone
Development

No branches or pull requests
6 participants
@moorereason @naveensrinivasan @spencerschrock @inferno-chromium @laurentsimon @afmarcum