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

make formatting of build-depends consistent #10714

Conversation

peterbecich
Copy link
Member

*.cabal files in this project presently have a mix of formatting of build-depends.

Make them all consistent with:
https://cabal.readthedocs.io/en/3.4/cabal-package.html

There are no changes to any dependencies.

Please read Github PR Conventions and then fill in one of these two templates.

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

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.

Seems fine to me. Maybe a candidate to .git-blame-ignore-revs?..

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!

@peterbecich
Copy link
Member Author

@ulysses4ever can this be merged?

@ffaf1 ffaf1 added squash+merge me Tell Mergify Bot to squash-merge and removed attention: needs-review labels Jan 12, 2025
@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Jan 12, 2025
@ffaf1
Copy link
Collaborator

ffaf1 commented Jan 12, 2025

I added the squash+merge me label @peterbecich. Let us sit tight for 48 hours and it will be auto-merged.

@peterbecich
Copy link
Member Author

@ffaf1 , incidentally, can manual merge be disabled? How can we configure this repo so only the Bot can merge?
image

@peterbecich
Copy link
Member Author

@ffaf1 , a second question, if you squash this PR and change the commit hash, that will break .git-blame-ignore-refs https://github.com/haskell/cabal/pull/10714/files#diff-f247fbfbe928b907db42554d0b3006b28dd11c25a59be031abda73b11a2c934a right?

@ffaf1 ffaf1 added merge me Tell Mergify Bot to merge and removed squash+merge me Tell Mergify Bot to squash-merge labels Jan 12, 2025
@ffaf1
Copy link
Collaborator

ffaf1 commented Jan 12, 2025

@ffaf1 , a second question, if you squash this PR and change the commit hash, that will break .git-blame-ignore-refs

Indeed, well spotted. I set it back to merge me.

incidentally, can manual merge be disabled? How can we configure this repo so only the Bot can merge?

Good question, I don't have an answer. I will ask in Matrix.

@ffaf1
Copy link
Collaborator

ffaf1 commented Jan 12, 2025

This was a reply I got, and I agree with it.

“multiple times in my experience something has broken CI such that we needed to manually merge something to fix it (usually some dependency). not sure restricting that to admins is wise, sometimes admins aren't around as with the holiday break[.]”

an more:

“I don't remember a single time when admins "were not around". We have plenty of admins these days. On the other hand, I remember plenty of accidental merges via the button by new or returning contributors. Hence, I am all for disabling the merge button for non-admins.”

@ulysses4ever
Copy link
Collaborator

Thanks for raising the issue about the merge button @peterbecich ! I'll try to raise this issue at the upcoming Cabal meeting (this Thursday, the 16th). I think it makes sense to restrict it to admins only.

@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jan 14, 2025
@Mikolaj Mikolaj force-pushed the fix-cabal-build-depends-formatting-inconsistency branch from 5c53f49 to fddf792 Compare January 14, 2025 07:29
@peterbecich
Copy link
Member Author

@Mikolaj remove fddf792 please

The commit hashes have changed so it has no purpose now, it is not critical.

@ffaf1
Copy link
Collaborator

ffaf1 commented Jan 14, 2025

It is an automated bot action, let me see if I can block the merge queue… Or we will just put the correct hash after the fact.

@Mikolaj
Copy link
Member

Mikolaj commented Jan 14, 2025

Maybe this is a rare case where manual merging is in order so that Mergify does not rebase and so invalidate the hashes. If so, @peterbecich, please feel free to use the button after rebasing manually and updating the commit with the hashes one last time.

@Mikolaj Mikolaj removed the merge me Tell Mergify Bot to merge label Jan 14, 2025
Copy link
Contributor

mergify bot commented Jan 14, 2025

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #10714 has been dequeued. The pull request rule doesn't match anymore. The following conditions don't match anymore:

  • label=merge me
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • any of: [🛡 GitHub branch protection]
        • check-neutral = Validate post job
        • check-skipped = Validate post job
        • check-success = Validate post job

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@ulysses4ever
Copy link
Collaborator

One merge option that fits this case is the "merge+no rebase" label, I believe. In that case Mergify won't touch the original commits. Of course, it's too little too late because the commits have been updated, so someone needs to update the SHA in the text file, and then put the "merge+no rebase" label...

@peterbecich peterbecich force-pushed the fix-cabal-build-depends-formatting-inconsistency branch from fddf792 to 72f37fd Compare January 17, 2025 01:44
`*.cabal` files in this project presently have a mix of formatting of `build-depends`.
Make them all consistent with:
https://cabal.readthedocs.io/en/3.4/cabal-package.html

No changes to any dependencies.
@peterbecich peterbecich force-pushed the fix-cabal-build-depends-formatting-inconsistency branch from 72f37fd to add05c4 Compare January 17, 2025 01:50
@peterbecich peterbecich merged commit 54b4838 into haskell:master Jan 17, 2025
50 checks passed
@peterbecich peterbecich deleted the fix-cabal-build-depends-formatting-inconsistency branch January 17, 2025 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants