Improved prePR

[zarthross]

I've found prePR to be a nice handy alias, but a little clumsy to extend. My team has been doing something similar for a while, but did a command alias in every project. I thought it would be nice if we had a handy easy way to extend the list of checks a developer should perform before submitting a pr so they have fewer painful moments realizing they forget a step and now their PR has failed.

[zarthross]

Happy to add documentation if it looks like this code change will get accepted.

[armanbilge]

Mostly curious, why did you introduce this setting in the kernel plugin? I think we could introduce and set the default in the CI plugin, without need a blank default.

Also the doc can be more specific that these are the steps run by the prePR command alias.

[zarthross]

Added it to kernel because at my work we are using the Github plugin without using TypelevelCiPlugin, so for our use it made more sense to push it into Kernel. Maybe there is an argument that we should be extending TypelevelCiPlugin... Ill take a look next week, but now that Ive looked closer at the CIPlugin it might make sense.

[armanbilge]

If you are only using the kernel plugin, then it's fairly straightforward to do whatever you want directly with tlCommandAliases.

[zarthross]

While it is, and I did... I thought I would basically push back up the changes I was making for myself internally. My thought process was that as people add different 'linters' or other things that are expected to pass in CI, such as scalafix/fmt, mima, or sbt-explicit-dependency, or sbt-missinglink , I wanted an easy way to aggregate those into a check at the the same time (in the same auto-plugin) as i was adding the github workflow.

What ended up happening in my world is people started defining a command alias called 'build' in each project and just put in the various steps, but somehow it would always be out of sync with what we actually did in CI.

[zarthross]

Alright, so the reason I'd prefer to keep prePR in kernel is because we have our own custom ci steps and we'd like the easier to extend prePR.

[zarthross]

@armanbilge Thoughts on getting this merged?

[armanbilge]

I'm pondering whether this belongs in kernel. Historically the mandate of kernel has been to implement public features that ideally would be upstreamed to sbt itself and private utilities that can be shared across the other plugins.

[zarthross]

@armanbilge I can make a separate plugin in kernel project or a separate project? In my opinion it doesn't belong in core either (we don't use core, we just use kernel, github, versioning, mima and scalafix)

[armanbilge]

In my opinion it doesn't belong in core either

I agree, I would say it belongs in the CI project—the notion of prePR is contingent on having some kind of repository with CI checks to merge into that repository.

[zarthross]

😅 Completely understand your reasoning, but I was intentionally trying to avoid putting it in the ci project because we don't want to depend on it... We don't want the TypelevelCiPlugin autoplugin on our classpath so it doesn't accidentally activate. We have our own Ci plugin we are implementing.

[zarthross]

@armanbilge We could also change it so instead of using prePR we use some other separate command alias with a new name for this? 'build', 'prepare', 'lint', 'check'.. just to name some.

[armanbilge]

We could also change it so instead of using prePR we use some other separate command alias with a new name for this? 'build', 'prepare', 'lint', 'check'.. just to name some.

These are all interesting and IMO "opinionated" which I think we should try to avoid for the kernel plugin.

[zarthross]

How about i make a new sbt project and move all the code in there? Something independent of prePR and named differently, but serves the purpose of having an aggregated set of steps a user should perform before submitting a PR to minimize the failures seen in CI when making a PR?

[armanbilge]

but serves the purpose of having an aggregated set of steps a user should perform before submitting a PR to minimize the failures seen in CI when making a PR?

That's exactly the purpose of prePR, which is why it's named such :) and for it to auto-populate it makes sense to be defined in the plugin that configures the CI.

I appreciate your effort to upstream but it sounds like you are looking for something custom and maybe sbt-typelevel is a bit too opinionated for you? Why can't you depend on the sbt-typelevel-ci plugin?

[zarthross]

Since our deployment pipeline is just too different from the sbt-typelevel-ci-plugin... If i included it i'd basically be overriding all the things it does anyways 😅.

Well, we can probably just do our own thing internally, but I can move this into the CI plugin for other projects to use? I still see the value for the OSS community either way.