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

Bump sbt-web-build-base to 2.0.1. Fixes #118 #119

Merged
merged 9 commits into from
Jan 13, 2024

Conversation

DavidLawes
Copy link
Contributor

@DavidLawes DavidLawes commented Oct 24, 2023

What does this change?

As discussed in #118 the latest version of sbt-less appears to be incompatible with the latest version of sbt-web. To resolve this we need to update the sbt-web-build-base version.

This change does a couple of things:

  • Bumps sbt-web-build-base to 2.0.1
  • The latest version of the base plugin allowed us to also bump versions of plugins sbt-js-engine and sbt-web
  • Bumps sbt to 1.9.6 (this is the minimum sbt version specified by sbt-web-build-base)
  • Given the latest version of sbt is used, SbtLess is refactored to use slash syntax
  • To ensure the tests can still be executed, the groupId of plugins used in tests have also been updated

How to test?

It appears as though this project makes use of the sbt scripted test framework for testing sbt plugins.

To test changes we can run sbt scripted.

I tested locally by doing the following:
* Publishing a version of the plugin to local maven (sbt publishM2)
* For each project inside ./sbt-test/sbt-less-plugin:
* Ensuring the project compiles and uses the locally published version of the plugin
* Execute the test instructions and confirm the expectations succeed

Note that some tests failed, e.g.:
image

I don't believe these errors are caused by this change because I believe this issue has already been reported and there is an open PR to resolve this. Please correct me if I'm wrong!

@@ -1,3 +1,3 @@
addSbtPlugin("com.typesafe.sbt" % "sbt-less" % sys.props("project.version"))
addSbtPlugin("com.github.sbt" % "sbt-less" % sys.props("project.version"))
Copy link
Contributor Author

@DavidLawes DavidLawes Oct 24, 2023

Choose a reason for hiding this comment

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

I'm not sure how often people contribute to this project but I had to do a little exploration to understand how best to test my changes. If you're happy for me to do so, I could add an explanatory note to CONTRIBUTING.md (about using scripted to execute the tests)?

Also, would you be open to receiving an additional PR to add a github action that executes the tests when a PR is raised?

Copy link
Contributor Author

@DavidLawes DavidLawes Oct 24, 2023

Choose a reason for hiding this comment

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

I think because this project extends from sbt-web-build-base this means that sbt-less will now also be published under the same (i.e. com.github.sbt/sbt-less). When publishing locally I can see:

image

As a consequence, will I also need to change this and/or anything else related to the release flow?

Copy link
Member

Choose a reason for hiding this comment

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

I didn't look yet, but please change the groupId of this plugin to com.github.sbt

Copy link
Member

Choose a reason for hiding this comment

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

Also, would you be open to receiving an additional PR to add a github action that executes the tests when a PR is raised?

I added GitHub actions workflows which run the scriptet tests and also for publishing to maven central here:

@DavidLawes
Copy link
Contributor Author

JFYI: I've received an email from Lightbend confirming I've signed the CLA

@mkurz
Copy link
Member

mkurz commented Oct 24, 2023

Thanks, I will take a look at this asap (probably not today)

@DavidLawes DavidLawes requested a review from mkurz October 30, 2023 14:38
@mkurz
Copy link
Member

mkurz commented Oct 30, 2023

I did not forget, give me some time.

@DavidLawes
Copy link
Contributor Author

Hi @mkurz - don't suppose you could take a look at this when you get a chance?

@mkurz
Copy link
Member

mkurz commented Nov 20, 2023

I want to get to sbt-web plugins this week actually, it's not just sbt-less but others as well (gzip, etc.)
It's on my list very high up, I will not forget about it.

* Bump sbt-web-build-base to 2.0.1
* Bump sbt-js-engine and sbt-web to latest
* Bump sbt to 1.9.6
* Refactor to slash syntax in SbtLess.scala
* Update groupId of plugins used in tests
Copy link
Member

@mkurz mkurz left a comment

Choose a reason for hiding this comment

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

Thanks! I rebased, pulled in

to fix the error that occured when running tests and and fixed other scripted tests.
I will set up a release process and publish a new release asap.

@mkurz mkurz merged commit 49de99f into sbt:main Jan 13, 2024
5 checks passed
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