Menu

#402 Automatically close bogus PRs

open
nobody
None
2021-02-07
2021-01-01
Anonymous
No

Originally created by: dscho

As mentioned in https://github.com/git/git/pull/937#issuecomment-752087607, we'd like to close PRs that are clearly bogus, such as PRs by new contributors trying to merge next into maint or anything similarly undesirable.

Some ideas for heuristics:

  • the base branch isn't in the contributor's fork
  • the patches aren't authored by the (not yet /allow'ed) contributor
  • a single patch adds new .c or .h or even .py file(s), without adjusting anything else
  • ...

Discussion

  • Anonymous

    Anonymous - 2021-01-01

    Originally posted by: dscho

    /cc @webstech

     
  • Anonymous

    Anonymous - 2021-01-04

    Originally posted by: tussi003

    Jak již bylo zmíněno v git / git # 937 (komentář) , rádi bychom, aby poblíž PRS, které jsou jasně falešné, jako jsou VAS nových přispěvatelů, kteří se snaží sloučit nextdo maintnebo něco podobně nežádoucí.

    Několik nápadů pro heuristiku:

    • základní větev není ve vidlici přispěvatele
    • opravy nejsou autorem (zatím / nepovoleným) přispěvatelem
    • jedna oprava přidává nové .cnebo .hdokonce .pysoubory, aniž by upravovala cokoli jiného
    • ...
     
  • Anonymous

    Anonymous - 2021-02-04

    Originally posted by: webstech

    Update: Ready to open PR

    Playing with this. This comment is tracking what is and isn't working. Running a test which analyzes the git/git open PRs and many of the closed bogus PRs. Reject means close the PR and warn means add a label to the PR. This is a WIP.

    Open PRs
    1. Three have changes to a single source and could be rejected

    Closed PRs
    1. Reject files with a space in the name
    2. Reject files with extensions longer than 8 characters
    3. Reject updates to certain filenames (or adding similarly named files) (eg README, SECURITy.md)
    4. Warn for files with no type
    5. Warn for files with small additions and no deletions (see open prs above)
    6. Warn for files with zero additions and multiple deletions (possibly deleted file)

    Current testing results:
    Checking 47 Open PRs


    ==> PR 924 warned: Label validate added for this reason:

    File builtin/init-db.c expecting more than one change.


    ==> PR 907 warned: Label validate added for this reason:

    File sha1dc/sha1.c expecting more than one change.


    ==> PR 859 warned: Label validate added for this reason:

    File userdiff.c expecting more than one change.


    ==> PR 857 warned: Label validate added for this reason:

    File .circleci/config.yml expecting more than one change.


    ==> PR 834 failed: PR is being closed:

    Could not resolve to a Repository with the name 'lukaspupkalipinski/git'.


    ==> PR 790 warned: Label validate added for this reason:

    File README.md not expected to be updated


    ==> PR 767 warned: Label validate added for this reason:

    File gitweb/README not expected to be updated


    ==> PR 765 failed: PR is being closed:

    File .gitmodules not allowed in root directory.


    Checking 34 Closed PRs

    ???> PR 953 true did not fail: Update sha1collisiondetection


    ==> PR 949 failed: PR is being closed:

    File .github/workflows/msbuild.ymlmarbbmervlinbisgmail.com not allowed with multi-part name


    ==> PR 947 warned: Label validate added for this reason:

    File IdiotaSECURITY.md not expected to be updated


    ==> PR 946 failed: PR is being closed:

    File SECURITY.mdmarvol120 has an invalid extension.


    ==> PR 940 failed: PR is being closed:

    Request author is not commit author.


    ==> PR 935 failed: PR is being closed:

    Request author is not commit author.


    ==> PR 933 failed: PR is being closed:

    File test_autotest not allowed in root directory.


    ==> PR 926 warned: Label validate added for this reason:

    File SECURITY.md not expected to be updated


    ???> PR 910 true did not fail: Simplified merge logic


    ==> PR 908 warned: Label validate added for this reason:

    File SECURITY.md not expected to be updated


    ???> PR 903 true did not fail: Update apply.h


    ==> PR 901 warned: Label validate added for this reason:

    File README.md not expected to be updated


    ???> PR 900 true did not fail: Update blob.c


    ==> PR 899 failed: PR is being closed:

    File dheeraj_gupta not allowed in root directory.


    ==> PR 893 failed: PR is being closed:

    File vowel consonant not allowed in root directory.


    ==> PR 892 failed: PR is being closed:

    File Binary search not allowed in root directory.


    ==> PR 891 failed: PR is being closed:

    File concatenate one string not allowed in root directory.


    ==> PR 889 failed: PR is being closed:

    File Array Duplicacy not allowed in root directory.


    ==> PR 888 failed: PR is being closed:

    File sum of arrays not allowed in root directory.


    ==> PR 886 failed: PR is being closed:

    File Swap2array not allowed in root directory.


    ==> PR 883 warned: Label validate added for this reason:

    File apply.c expecting more than one change.


    ==> PR 868 failed: PR is being closed:

    Request author is not commit author.


    ==> PR 860 warned: Label validate added for this reason:

    File SECURITY.md not expected to be updated


    ==> PR 858 warned: Label validate added for this reason:

    File SECURITY.md not expected to be updated


    ==> PR 849 failed: PR is being closed:

    File capstone-project not allowed in root directory.


    ==> PR 845 failed: PR is being closed:

    File capstone-project not allowed in root directory.


    ==> PR 840 failed: PR is being closed:

    Request author is not commit author.


    ==> PR 839 warned: Label validate added for this reason:

    File README.md not expected to be updated


    ==> PR 813 warned: Label validate added for this reason:

    File SECURITY.md not expected to be updated


    ==> PR 794 warned: Label validate added for this reason:

    File .gitignore expecting more than one change.


    ==> PR 789 failed: PR is being closed:

    File gitpod not allowed in root directory.


    ==> PR 786 warned: Label validate added for this reason:

    File .github/workflows/main.yml expecting more than one change.


    ==> PR 783 failed: PR is being closed:

    File Drygo1992 not allowed in root directory.


     
  • Anonymous

    Anonymous - 2021-02-06

    Originally posted by: dscho

    I like the idea of testing for expected file extensions, and for the requestor not being the author (for new users; I often contribute on others' behalf).

    Not so sure about disallowing SECURITY.md: It would actually be good for us to have one.

    Likewise, I'm unsure about .gitmodules: we do unfortunately have a submodule.

    And disallowing a single change would probably have too many false hits: typo fixes are valuable, for example.

     
  • Anonymous

    Anonymous - 2021-02-07

    Originally posted by: webstech

    Not so sure about disallowing SECURITY.md: It would actually be good for us to have one.

    There are a number of PRs that included bogus SECURITY.md (or similarly named) files so it was included but not as a closer.

    And disallowing a single change would probably have too many false hits: typo fixes are valuable, for example.

    The PR adds a label for a number of situations. It could be changed to always add a label for now. In that situation, only a new user PR that appears to touch several files would not be flagged. Really do not want to close valid PRs.

    Noting this problem appears to be happening with git/git and not with gitgitgadget/git.

     
  • Anonymous

    Anonymous - 2021-02-07

    Originally posted by: webstech

    This is the Jest test for testing open PRs and select closed PRs. The check for closed has to be turned off in ci-helper.ts. The list of closed PRs is generally what would be considered to be 'bogus PRs'.
    newuser.test.ts.zip

     

Log in to post a comment.