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

Feature (Question) Git merging strategy #4454

Closed
kikofernandez opened this issue Dec 12, 2024 · 2 comments
Closed

Feature (Question) Git merging strategy #4454

kikofernandez opened this issue Dec 12, 2024 · 2 comments
Labels
kind/enhancement New feature or request

Comments

@kikofernandez
Copy link
Contributor

Is your feature request related to a problem? Please describe.

There are some repos that use the following git merging model, where scorecard downgrades the score, possibly for valid reasons, but I would like to ask.

  • A repo has two main branches, maint for doing minor scheduled releases on the current version, and next-release for adding changes to the next major release.
  • Everything that goes into maint is in a PR and reviewed, and the same model applies for PRs towards next-release.
  • When reviewing and merging a PR in maint, those changes must also be merged to the next-release branch, so one must checkout next-release and merge maint into next-release, to keep next-release in sync with maint.
  • Merging maint into next-release without opening a PR cause scorecard to not considered the merge as reviewed, and
  • it would be extremely cumbersome and not maintainable to open a PR for every merged PR of maint that needs to be applied to next-release, when the content is exactly the same.

My question is:

  • is there a way for scorecard to understand that the PR was reviewed, and got immediately merged into the default main branch (next-release)?
  • Does this make sense for scorecard or is it considered a security fault?
    Thanks!

Describe the solution you'd like
I would like to understand if it is an option for scorecard to detect that a PR that has been reviewed and merged needs to be reviewed again to be merged against the next-release branch.

Describe alternatives you've considered

  • the main branch here is next-release and we cannot change it to be maint due to other implications, so that cannot help.
  • I am not sure that we can change the way merges are done... we can do (and actually do) atomic pushes to maint and next-release to guarantee that we do not merge changes in maint that lead to conflicts in next-release, but this is a local operation and Github cannot validate it.

Additional context
I know that the merged PR into maint could cause conflicts when maint gets merged into next-release, but this is rarely an issue in practice (AFAIK).
@kikofernandez kikofernandez added the kind/enhancement New feature or request label Dec 12, 2024
@spencerschrock
Copy link
Member

I'll start off by saying Scorecard doesn't capture every developer workflow well.

is there a way for scorecard to understand that the PR was reviewed, and got immediately merged into the default main branch (next-release)?

as it stands right now, Scorecard only considers the default branch (which you pointed out in your issue) and does code review in one of the following ways:

  • Detecting an associated PR using GitHub's API
  • Detects an external review platform from the commit message.

Does this make sense for scorecard or is it considered a security fault?

In my opinion, the functional changes were still reviewed, which is in the spirit of the check (and part of the reason we look for reviews in external platforms (Gerrit, Phabricator etc). In terms of Scorecard counting this, there would be some challenges:

  • how to detect this.
  • the contents of the merge commit itself may not be reviewed. I know GitHub as a minified view for some merge commits (to see if anything extra was added), but I'm not sure how accessible that would be to scorecard.

@kikofernandez
Copy link
Contributor Author

Ok, thanks for the response. I will keep thinking about how to detect this, unless there is a 100% probably to not accept this detection :)
I close the issue and will re-open if I get something in a PR.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/enhancement New feature or request
Projects
Scorecard - NEW
Status: Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@kikofernandez @spencerschrock