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

Futz with the build #409

Merged
merged 2 commits into from
Mar 23, 2022
Merged

Conversation

armanbilge
Copy link
Contributor

No description provided.

Comment on lines -18 to -22
ThisBuild / homepage := Some(url("https://github.com/TimWSpence/cats-stm"))

ThisBuild / scmInfo := Some(
ScmInfo(url("https://github.com/TimWSpence/cats-stm"), "git@github.com:TimWSpence/cats-stm.git")
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is set automatically. Actually with more info then you've set here I believe 😆

Comment on lines -62 to -64
.settings(
javacOptions ++= Seq("-source", "1.8", "-target", "1.8")
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You don't have java code in your project do you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw this will also be set automatically in a future release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, you do have Java.

ThisBuild / scmInfo := Some(
ScmInfo(url("https://github.com/TimWSpence/cats-stm"), "git@github.com:TimWSpence/cats-stm.git")
)
ThisBuild / tlJdkRelease := Some(8)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There you go :)

Copy link
Owner

Choose a reason for hiding this comment

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

Does this set the javac target option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, actually no :) it sets the --release flag which has superseded that, but anyway.

Copy link
Owner

Choose a reason for hiding this comment

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

Even better, thanks!

@TimWSpence
Copy link
Owner

Sorry @armanbilge, just saw this! I thought I had approved it to run the build but it doesn't seem to have done it...

@armanbilge armanbilge closed this Mar 22, 2022
@armanbilge armanbilge reopened this Mar 22, 2022
@@ -34,6 +29,7 @@ lazy val `cats-stm` = tlCrossRootProject
core,
benchmarks,
docs,
unidoc,
Copy link
Owner

Choose a reason for hiding this comment

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

What is going on with this diff? That is already aggregated on main

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol, I opened this PR before you did that, and it doesn't detect the merge conflict.

Anyway, nothing critical in here, just some things I noticed while looking at your build. Also you'll still need to apply add the settings from typelevel/sbt-typelevel#214 (comment) to fix your publishing.

Copy link
Owner

Choose a reason for hiding this comment

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

Haha yeah, it's on my TODO list. Thanks!

@TimWSpence
Copy link
Owner

Thanks so much @armanbilge! :)

@TimWSpence TimWSpence merged commit 47cc7bc into TimWSpence:main Mar 23, 2022
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.

2 participants