-
Notifications
You must be signed in to change notification settings - Fork 490
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
Check up to 5 nested project folders #2995
Conversation
ce44d63
to
9ab8af3
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2995 +/- ##
==========================================
+ Coverage 90.98% 91.02% +0.04%
==========================================
Files 160 160
Lines 3305 3322 +17
Branches 297 303 +6
==========================================
+ Hits 3007 3024 +17
Misses 298 298
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
Would be nice if this PR makes it into v0.23.0 😉 |
Hey! I am pretty sure this pull request is good to go, reviews would be highly appreciated. Thanks! |
Thanks for the PR, @mkurz! I'm fine with updating dependencies in meta-meta-builds and even meta-meta-meta-builds but I think it would be better to check how many nested I would check how many (nested) |
I get it. Will update when I find time, thanks! |
9ab8af3
to
ae2a739
Compare
@fthomas Updated. It works exactly like you asked for, depending on the levels of meta builds. I tested that again on a real project: https://github.com/mkurz/three-level-sbt-plugins-project/pulls (I deleted the repo and recreated it so it was fresh again). As you can see a pull request for each meta build was created. After one run I even created one more level with the scalafmt plugin and on the second run scala-steward also created the according pull request (you can see that the sbt-scalafmt pull request was opened 4 minutes later). |
.iterate(buildRootDir / project)(_ / project) | ||
.takeWhile(p => p.exists && p.isDirectory) | ||
.size, | ||
1 // There is always at least one meta build, even if there is no project folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we should drop this Math.max
call. If there is no project
folder, there aren't any *.sbt
files either which could contain dependencies (plugins) that Scala Steward could update. And if there is nothing to update, why loading the meta-build and calling stewardDependencies
? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory yes. However removing Math.max
will break many many tests because of lines like
scala-steward/modules/core/src/test/scala/org/scalasteward/core/buildtool/sbt/SbtAlgTest.scala
Line 29 in a6ff326
Cmd("write", s"$repoDir/project/project/scala-steward-StewardPlugin_1_3_11.scala"), |
scala-steward/modules/core/src/test/scala/org/scalasteward/core/buildtool/sbt/SbtAlgTest.scala
Line 43 in a6ff326
Cmd("rm", "-rf", s"$repoDir/project/project/scala-steward-StewardPlugin_1_3_11.scala"), |
which assume scala-steward always checks/updates the project
folder. And I would keep all those tests how they are currently.
Also in real life I never saw a sbt project without a project
folder (sbt even creates the project
folder when running in an empty folder).
I would just keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I would keep all those tests how they are currently.
We can keep them if we use FileAlg#isDirectory
for checking the project
folders as I've done in 1a97fc5. Another benefit of using FileAlg
is that the tests are now sensitive to the actual number of project
folders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks!
Thanks @mkurz! |
This allows to remove the `Math.max(..., 1)` call since our tests actually count the `project` directories that are present in `MockState`.
LGTM 👍 |
Tested again with a real world repo, everything works great: https://github.com/mkurz/three-level-sbt-plugins-project/pulls |
Awesome, thanks for testing again! |
Some projects set up plugins in
project/project/plugins.sbt
and even inproject/project/project/plugins.sbt
(I worked on such a private project, so I can't share details, but such things do exists 😉)For example, Play and the sbt-web-build-base project put sbt-buildinfo in their
project/project/plugins.sbt
files:However, as you can see, even though scala-steward is enabled and visiting those repos, that plugin never gets updated (latest release is 0.11.0, also see maven central)
Making scala-steward check and update those dependencies is actually pretty easy and straight forward:
We just need to set up the special
scala-steward-StewardPlugin_<version>.scala
plugin in the correspondingproject/project/project/project/
andproject/project/project/
folders so they are available as plugins for the parent "build" and then just enter those sub "build" projects viareloadPlugins
again and also runstewardDependencies
which also logs the dependencies of those projects.Nothing special, just a couple of more lines logged and parsed by scala steward.
I also think the test that I adjusted in this pull request explains everything.
To make sure everything works 100% I set up following test repo:
As you can see it has three levels of
project
folders includingplugins.sbt
:So I added outdated plugins to each
plugins.sbt
and thebuild.sbt
and ran scala steward with this pull request applied locally and voilà, scala steward opened all expected pull requests:BTW:
If you want to test out what the output of the sbt command ran by sbt looks like, you can check out the
steward-plugin-file
branch of my test repo and ran the sbt command I show in the README. It is exactly what scala steward runs to figure out the versions. (For this tostewardDependencies
command to work I manually added theStewardPlugin_1_3_11.scala
files myself, see the README again).