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

The uploaded SARIF doesn't seem to be always reflected in PRs #19

Open
praiskup opened this issue Jun 21, 2023 · 6 comments
Open

The uploaded SARIF doesn't seem to be always reflected in PRs #19

praiskup opened this issue Jun 21, 2023 · 6 comments

Comments

@praiskup
Copy link
Member

E.g. rpm-software-management/mock#1103

  1. Ci is Failed: "Lint Python issues / python-lint-job (pull_request) Failing after 40s" task
  2. Log says
    ## Linting in 'mock' subdirectory ##
    # ================
    #  Added warnings 
    # ================
    
    Error: PYLINT_WARNING:
    py/mockbuild/buildroot.py:11: W0611[unused-import]: Unused import shlex
    
    Error: PYLINT_WARNING:
    py/mockbuild/buildroot.py:22: W0611[unused-import]: Unused RootError imported from exception
    

But there are no comments "in-line" about those.

@jamacku
Copy link
Collaborator

jamacku commented Jun 26, 2023

I have noticed a similar thing with differential-shellcheck. In code with "more" defects, GitHub sometimes doesn't show some alerts in UI.

It is related to the recent introduction of GitHub "differential" scanning. GitHub tries to show only relevant alerts, but its algorithm could be better. They are using SARIF fingerprints.

Please see @kdudka comment: csutils/csdiff#98 (comment)

@praiskup
Copy link
Member Author

So does the hash content actually matter? We never report other than new errors; except that it would be useful to have something meaningful long-term, would generating random hashes just help us to work around the problem?

@jamacku
Copy link
Collaborator

jamacku commented Jun 26, 2023

The implementation of fingerprints in csdiff might help with this issue. But it's quite challenging to make it right.

Maybe generating some random hashes for fingerprints might be a workaround, but I don't know how relatable.

@praiskup
Copy link
Member Author

praiskup commented Sep 30, 2024

Similar problem, it seems that GitHub doesn't show new issues that appear in code that has not been updated. A notable example -- I drop a method call foo() from python file, which makes the from baz import foo statement above useless.

Pylint shouts to us loudly, csdiff correctly detects this as a new issue, vcs-diff-lint propagates this as new issue, but GitHub swallows the report because it appears in part of the source file that has not been modified.

@praiskup
Copy link
Member Author

praiskup commented Oct 1, 2024

Example: fedora-copr/vcs-diff-lint#35

@praiskup
Copy link
Member Author

praiskup commented Oct 7, 2024

I tried to report this against GitHub: https://github.com/orgs/community/discussions/140754

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

No branches or pull requests

2 participants