Skip to content

ci-commitmessage-submodules: Ignore empty merge commits#2079

Merged
openshift-merge-robot merged 1 commit intocoreos:masterfrom
jlebon:pr/skip-merges-ci
May 7, 2020
Merged

ci-commitmessage-submodules: Ignore empty merge commits#2079
openshift-merge-robot merged 1 commit intocoreos:masterfrom
jlebon:pr/skip-merges-ci

Conversation

@jlebon
Copy link
Copy Markdown
Member

@jlebon jlebon commented May 6, 2020

Jenkins does its own git merge when testing PRs. Doing a naive
git diff ${merge_commit}^..${merge_commit} won't work right because
it might perform a diff across multiple commits.

What we want to do here is to just skip trivial merge commits or
otherwise error out on them if they're non-trivial (since it likely
means that one did conflict resolution manually instead of rebasing,
which we should encourage).

The origin/master..$HEAD range will correctly still contain all the
parents of any merge commit which is not yet in origin/master.

@jlebon jlebon force-pushed the pr/skip-merges-ci branch 4 times, most recently from c483a48 to 8ea89cc Compare May 6, 2020 16:41
@jlebon jlebon changed the title WIP: ci-commitmessage-submodule: Ignore Jenkins' merge ci-commitmessage-submodules: Ignore empty merge commits May 6, 2020
@jlebon jlebon marked this pull request as ready for review May 6, 2020 16:41
@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented May 6, 2020

OK, this should be ready now! Should allow CI to stop breaking all the PRs every time there's a libdnf bump.

@cgwalters
Copy link
Copy Markdown
Member

/approve
/lgtm

Jenkins does its own `git merge` when testing PRs. Doing a naive
`git diff ${merge_commit}^..${merge_commit}` won't work right because
it might perform a diff across multiple commits.

What we want to do here is to just skip trivial merge commits or
otherwise error out on them if they're non-trivial (since it likely
means that one did conflict resolution manually instead of rebasing,
which we should encourage).

The `origin/master..$HEAD` range will correctly still contain all the
parents of any merge commit which is not yet in `origin/master`.
@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented May 6, 2020

Ahh heh. The commit message for this commit itself had a line that started with parent and messed up the grep. Anyway, I switched to something cleaner based on https://stackoverflow.com/questions/3824050#comment82244548_13956422.

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented May 6, 2020

Will download: 2 packages (639.9?kB)
Downloading from 'fedora-coreos-pool'...done
error: Cannot download Packages/o/ostree-libs-2020.3-2.fc31.x86_64.rpm: All mirrors were tried; Last error: Curl error (6): Couldn't resolve host name for https://kojipkgs.fedoraproject.org/repos-dist/coreos-pool/latest/x86_64/Packages/o/ostree-2020.3-2.fc31.x86_64.rpm [Could not resolve host: kojipkgs.fedoraproject.org]

Looks like other PRs are hitting this too. Investigating.

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented May 7, 2020

Looks like other PRs are hitting this too. Investigating.

OK, so basically the overlap of cosa overrides + lockfiles needs some more ironing out. I filed coreos/coreos-assembler#1431 for now, but I started on a patch to fix this.

@jlebon
Copy link
Copy Markdown
Member Author

jlebon commented May 7, 2020

✔️ continuous-integration/jenkins/pr-merge — This commit looks good

🎉

@cgwalters
Copy link
Copy Markdown
Member

/lgtm

@openshift-ci-robot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit fc5825a into coreos:master May 7, 2020
jlebon added a commit to jlebon/ostree that referenced this pull request Jun 17, 2020
@jlebon jlebon deleted the pr/skip-merges-ci branch April 23, 2023 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants