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

refactor(ci): switch from setup-scala to setup-java #242

Merged
merged 1 commit into from
Nov 15, 2022

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Nov 15, 2022

This updates to using setup-java which is more actively maintained and recommended now than setup-scala. Plus this has built-in sbt caching. In the process of this I've also migrated from master -> main.

@kitbellew
Copy link
Contributor

  1. would like @olafurpg to chime in, as the original author of the setup-scala plug-in
  2. does the change from master to main mean the logic didn't work before?

@ckipp01
Copy link
Member Author

ckipp01 commented Nov 15, 2022

would like @olafurpg to chime in, as the original author of the setup-scala plug-in

I don't really think that's necessary. He's publicly said

I recommend using coursier/setup-action or setup-java. I’m considering archiving setup-scala since it’s been superseded by better alternatives.

We've also done this in other Scalameta projects like Metals as you can see here.

does the change from master to main mean the logic didn't work before?

No it doesn't mean that it didn't work, but since I was making changes there I thought it a good idea to be better in line with the Scala Inclusive Language Guide by avoiding loaded words like master. This guide is mentioned in Scala COC which is what all of our Scalameta projects follow.

@kitbellew
Copy link
Contributor

are you saying that the primary branch was just renamed via some process? in that case, why didn't that process also rename in the .yml files?

generally, i'd recommend making the rename a completely separate, unrelated change which doesn't get accidentally rolled back if the rest of the pr somehow fails to perform.

@ckipp01
Copy link
Member Author

ckipp01 commented Nov 15, 2022

are you saying that the primary branch was just renamed via some process?

The primary branch needs to be manually changed in the settings (which I've already done). So the pr to change this would be simply changing it to main in the three files. Again, since those files were going to be changed in this pr I just grouped it all together. If something breaks I'll happily address it when we merge, but the CI ran successfully, meaning that it's probably ok.

@kitbellew
Copy link
Contributor

are you saying that the primary branch was just renamed via some process?

The primary branch needs to be manually changed in the settings (which I've already done). So the pr to change this would be simply changing it to main in the three files. Again, since those files were going to be changed in this pr I just grouped it all together. If something breaks I'll happily address it when we merge, but the CI ran successfully, meaning that it's probably ok.

thank you for the explanation. if ungrouping the two changes is not too difficult, i would appreciate it. perhaps i could also do it myself if you prefer.

@ckipp01
Copy link
Member Author

ckipp01 commented Nov 15, 2022

thank you for the explanation. if ungrouping the two changes is not too difficult, i would appreciate it. perhaps i could also do it myself if you prefer.

Sure, I opened #243. After that's merged in I'll rebase this one to only focus on the setup-java change.

@kitbellew
Copy link
Contributor

thank you for the explanation. if ungrouping the two changes is not too difficult, i would appreciate it. perhaps i could also do it myself if you prefer.

Sure, I opened #243. After that's merged in I'll rebase this one to only focus on the setup-java change.

thank you very much. #243 has been merged.

This updates to using setup-java which is more actively maintained and
recommended now than setup-scala. Plus this has built-in sbt caching.
@ckipp01 ckipp01 merged commit 6ea38b3 into scalameta:main Nov 15, 2022
@ckipp01 ckipp01 deleted the ci branch November 15, 2022 19:22
kitbellew added a commit to kitbellew/scalafmt that referenced this pull request Dec 10, 2022
kitbellew added a commit to kitbellew/scalafmt that referenced this pull request Dec 10, 2022
kitbellew added a commit to kitbellew/scalafmt that referenced this pull request Dec 10, 2022
kitbellew added a commit to kitbellew/scalafmt that referenced this pull request Dec 10, 2022
kitbellew added a commit to kitbellew/scalafmt that referenced this pull request Dec 11, 2022
kitbellew added a commit to scalameta/scalafmt that referenced this pull request Dec 11, 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