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

Add plugin for generating .mergify.yml config #190

Merged
merged 24 commits into from
Mar 1, 2022

Conversation

armanbilge
Copy link
Member

This was originally proposed in sbt/sbt-github-actions#46.

Essentially, the problem is that in order to validate CI success the mergify config needs a list of all the jobs in the CI matrix with their exact names (including the Scala version) (mergify docs on this topic). In practice, manually keeping these in sync is annoying, and sbt-gh-actions is managing the build matrix anyway.

By default, the plugin includes a configuration to merge CI-passing PRs from scala-steward for patch updates. This can be further configured to include minor bumps and/or disabled.

Another idea I have is to generate config for auto-labeling of PRs based on file paths of affected modules, like http4s does. It'll be nice not to have to hand-maintain that.

Since this plugin is new, I'm keeping it as a separate module that has to be added explicitly to a build. It's still lacking DSL for many mergify features, but we can add them as required. It currently supports merging and labeling which are probably the most important.

Once it's proven itself we can consider if we want to include it in the sbt-typelevel super-plugin.

Comment on lines +9 to +16
- name: merge scala-steward's PRs
conditions:
- author=scala-steward
- or:
- body~=labels:.*early-semver-patch
- body~=labels:.*early-semver-minor
- status-success=Build and Test (ubuntu-latest, 2.12.15, temurin@8, rootJVM)
- '#approved-reviews-by>=1'
Copy link
Member Author

Choose a reason for hiding this comment

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

@fthomas if you have a chance, would appreciate your eyes on this. Are these labels the right configuration for checking steward PRs? Thanks! :)

Comment on lines +14 to +15
ThisBuild / mergifyStewardConfig ~= { _.map(_.copy(mergeMinors = true)) }
ThisBuild / mergifySuccessConditions += MergifyCondition.Custom("#approved-reviews-by>=1")
Copy link
Member Author

Choose a reason for hiding this comment

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

mergeMinors, because I'm dog-fooding. But I actually don't trust any update to upstream sbt plugins is not going to break things, patch or not ...

.mapJson(l => Json.obj("label" -> l))
}

private[this] object Dummy extends MergifyAction
Copy link
Member Author

Choose a reason for hiding this comment

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

This trick lets us keep MergifyAction sealed while being able to compatibly add additional cases to the ADT, since users will never be able to exhaustively pattern match.

@armanbilge
Copy link
Member Author

Now, the question is how can I get mergify working on this repo 🤔

@armanbilge
Copy link
Member Author

@Mergifyio refresh

@armanbilge armanbilge added this to the v0.4.6 milestone Mar 1, 2022
@armanbilge armanbilge marked this pull request as ready for review March 1, 2022 18:31
.mergify.yml Outdated
remove: []
- name: Label github PRs
conditions:
- files~=^github
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, this should end in /, or else it will pick up changes in github-actions as well.

@armanbilge
Copy link
Member Author

This is a whopper, but the generated mergify config is valid (correct, idk 🙃). Once it's out in the wild we'll understand better how it works. I'll keep it stable for 0.4 but if necessary will change some stuff around in 0.5.

@nafg
Copy link

nafg commented Mar 2, 2022

You could have just used sbt-mergify-github-actions from https://github.com/nafg/mergify-yaml/

@nafg
Copy link

nafg commented Mar 2, 2022

It autogenerates the case classes from the Mergify docs so it's complete

@armanbilge
Copy link
Member Author

@nafg cool! The hardest part while working on this one was expanding the matrix.

private lazy val jobSuccessConditions = Def.setting {
githubWorkflowGeneratedCI.value.flatMap {
case job if mergifyRequiredJobs.value.contains(job.id) =>
GenerativePlugin
.expandMatrix(
job.oses,
job.scalas,
job.javas,
job.matrixAdds,
job.matrixIncs,
job.matrixExcs
)
.map { cell =>
MergifyCondition.Custom(s"status-success=${job.name} (${cell.mkString(", ")})")

Would appreciate any PRs to help with the case classes ❤️

@nafg
Copy link

nafg commented Mar 2, 2022

Not sure I follow what you mean about expanding the matrix.

But you could use mergify-writer as standalone library if you like. Like I said the case classes are autogenerated from Mergify docs.

Or you could submit a PR improving the logic to my SBT plugin ;)

Or if it's explained to me (and it doesn't already do it) then I could add it myself.

@joroKr21
Copy link
Member

How do I set it up? It looks like just adding the config is not enough. I also tried to install it from here: https://github.com/apps/mergify/installations/new but it's not working: typelevel/cats-tagless#336

@armanbilge
Copy link
Member Author

@joroKr21 I also had trouble setting it up on my Typelevel repositories. But it seems to be working well in http4s/http4s-jdk-http-client#598. I think we need an org admin to enable it for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants