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

CI: install changelog-d from bindist #10048

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

fgaz
Copy link
Member

@fgaz fgaz commented May 23, 2024

This will avoid build problems when the GHC in the CI environment is updated sooner than expected.
Previous breakage: #9177 (comment)


Template B: This PR does not modify behaviour or interface

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

And this should be expedited (exempt from the two-days delay).

@ulysses4ever ulysses4ever added attention: needs-review attention: needs-backport 3.12 merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels May 23, 2024
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@ulysses4ever
Copy link
Collaborator

@mergify backport 3.12

Copy link
Contributor

mergify bot commented May 23, 2024

backport 3.12

✅ Backports have been created

@ulysses4ever ulysses4ever added the merge me Tell Mergify Bot to merge label May 23, 2024
@ulysses4ever
Copy link
Collaborator

@Mergifyio refresh

This will avoid build problems when the GHC in the CI environment
is updated sooner than expected.
Previous breakage: haskell#9177 (comment)
@Mikolaj Mikolaj force-pushed the changelog-d-binary branch from 55625cc to 4f85c1d Compare May 23, 2024 19:40
Copy link
Contributor

mergify bot commented May 23, 2024

refresh

✅ Pull request refreshed

@fgaz
Copy link
Member Author

fgaz commented May 23, 2024

It's one commit so we can squash, no?

@fgaz fgaz added pr: squash PR should be squashed upon merge merge me Tell Mergify Bot to merge and removed merge me Tell Mergify Bot to merge labels May 23, 2024
@haskell haskell deleted a comment from mergify bot May 23, 2024
@haskell haskell deleted a comment from mergify bot May 23, 2024
@ulysses4ever
Copy link
Collaborator

pr:squash label doesn't do anything magical, it's just a reminder to squash manually if the branch is protected and the bot can't do that by itself. But in this case, no squashing is needed exactly because there's just one commit (and the branch is not protected either). So, the label isn't needed either.

Current status: the branch was rebased on master by the bot, and the CI has to re-run and complete before the bot will merge it to master. If no one removes the necessary labels (merge-me, merge delay passed) that is.

@fgaz fgaz added squash+merge me Tell Mergify Bot to squash-merge and removed merge me Tell Mergify Bot to merge pr: squash PR should be squashed upon merge labels May 23, 2024
@fgaz
Copy link
Member Author

fgaz commented May 23, 2024

Yeah sorry I made a bit of a mess with labels. I think it makes sense to also squash-merge prs with a single commit, since there isn't anything to group up, but feel free to revert the label change

@ulysses4ever
Copy link
Collaborator

I don't know if 1-commit PRs squashed look the same as merged. They probably are, and in that case, it doesn't matter which label to use.

@geekosaur
Copy link
Collaborator

geekosaur commented May 23, 2024

The squash per se does nothing since there's nothing to squash. The only significant difference is whether it does a merge or a rebase to get it into master, and IIRC both labels use the same strategy.

ETA: looks like our squash+merge me uses merge instead of rebase as merge me does. But the comment on it says that the squash part makes them do the same thing.

@fgaz
Copy link
Member Author

fgaz commented May 23, 2024

I don't know if 1-commit PRs squashed look the same as merged. They probably are, and in that case, it doesn't matter which label to use.

They look different: squash+merge will result in one commit with (#10048) appended to the first line of the commit message, while merge will result in two commits: the one being merged and the merge commit Merge #10048 into master

@ulysses4ever
Copy link
Collaborator

squash+merge will result in one commit with (#10048) appended to the first line of the commit message, while merge will result in two commits: the one being merged and the merge commit Merge #10048 into master

Oh, I prefer the former very much. I wish I understood why it works like that. Also, can we set it up so that it never creates these ugly merge commit? (Or am I the only one who doesn't like them?..)

looks like our squash+merge me uses merge instead of rebase as merge me does

hmm, this looks like it should behave the opposite to what fgaz describes in the quote above?.. I'm so confused!

@geekosaur
Copy link
Collaborator

I noticed that as well.

What I see in the git log is that merge me results in merge commits and squash+merge me doesn't (as @fgaz says), despite what mergify.yml says. I presume this means that mergify.yml is referring to Mergify's strategy, not the resulting git strategy. This may mean we want to switch the strategy used when merge me is specified.

@ulysses4ever
Copy link
Collaborator

Yes!

geekosaur added a commit to geekosaur/cabal that referenced this pull request May 23, 2024
Per haskell#10048 (comment)
ff.

In studying the Mergify documentation, I discovered the actual rules are
slightly different than the ones we thought it was using, so I used the
ones the documentation cited (cf.
https://docs.mergify.com/workflow/actions/merge/#parameters).
@mergify mergify bot merged commit d1a6ced into haskell:master May 23, 2024
50 of 51 checks passed
mergify bot pushed a commit that referenced this pull request May 23, 2024
This will avoid build problems when the GHC in the CI environment
is updated sooner than expected.
Previous breakage: #9177 (comment)

(cherry picked from commit d1a6ced)

# Conflicts:
#	.github/workflows/changelogs.yml
@fgaz fgaz deleted the changelog-d-binary branch May 24, 2024 06:47
geekosaur added a commit to geekosaur/cabal that referenced this pull request May 25, 2024
Per haskell#10048 (comment)
ff.

In studying the Mergify documentation, I discovered the actual rules are
slightly different than the ones we thought it was using, so I used the
ones the documentation cited (cf.
https://docs.mergify.com/workflow/actions/merge/#parameters).
wismill pushed a commit to wismill/cabal that referenced this pull request May 26, 2024
This will avoid build problems when the GHC in the CI environment
is updated sooner than expected.
Previous breakage: haskell#9177 (comment)
geekosaur added a commit to geekosaur/cabal that referenced this pull request May 26, 2024
Per haskell#10048 (comment)
ff.

In studying the Mergify documentation, I discovered the actual rules are
slightly different than the ones we thought it was using, so I used the
ones the documentation cited (cf.
https://docs.mergify.com/workflow/actions/merge/#parameters).
geekosaur added a commit to geekosaur/cabal that referenced this pull request May 26, 2024
Per haskell#10048 (comment)
ff.

In studying the Mergify documentation, I discovered the actual rules are
slightly different than the ones we thought it was using, so I used the
ones the documentation cited (cf.
https://docs.mergify.com/workflow/actions/merge/#parameters).

We now use `squash` instead of `rebase`, so the PR number is preserved
for `changelog-d` to find.
geekosaur added a commit to geekosaur/cabal that referenced this pull request May 26, 2024
Per haskell#10048 (comment)
ff.

In studying the Mergify documentation, I discovered the actual rules are
slightly different than the ones we thought it was using, so I used the
ones the documentation cited (cf.
https://docs.mergify.com/workflow/actions/merge/#parameters).

We now use `squash` instead of `rebase`, so the PR number is preserved
for `changelog-d` to find.
wismill pushed a commit to wismill/cabal that referenced this pull request May 27, 2024
This will avoid build problems when the GHC in the CI environment
is updated sooner than expected.
Previous breakage: haskell#9177 (comment)
geekosaur added a commit to geekosaur/cabal that referenced this pull request May 31, 2024
Per haskell#10048 (comment)
ff.

In studying the Mergify documentation, I discovered the actual rules are
slightly different than the ones we thought it was using, so I used the
ones the documentation cited (cf.
https://docs.mergify.com/workflow/actions/merge/#parameters).

We now use `squash` instead of `rebase`, so the PR number is preserved
for `changelog-d` to find.
mergify bot added a commit that referenced this pull request Jun 7, 2024
* CI: install changelog-d from bindist (#10048)

This will avoid build problems when the GHC in the CI environment
is updated sooner than expected.
Previous breakage: #9177 (comment)

(cherry picked from commit d1a6ced)

# Conflicts:
#	.github/workflows/changelogs.yml

* !fixup resolve conflicts

---------

Co-authored-by: Francesco Gazzetta <[email protected]>
Co-authored-by: Artem Pelenitsyn <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
continuous-integration merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days squash+merge me Tell Mergify Bot to squash-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants