-
Notifications
You must be signed in to change notification settings - Fork 31
chore(ci): align usage of setup-java #484
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
Conversation
jobs: | ||
scip: | ||
runs-on: ubuntu-latest | ||
name: "Upload SCIP" | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: olafurpg/setup-scala@v13 | ||
- uses: actions/setup-java@v3 | ||
- uses: actions/setup-go@v2 | ||
with: | ||
go-version: "1.15.6" |
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.
Is go actually needed? I know in my uploads I don't include this and it seems to work fine?
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.
This might be a remnant from the time when we build src
from source. If CI is green without this step then it's fine to remove 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.
👍🏼 I'll send in another pr after this one and double-check. I don't want to edit this one again because I opened it before my first pr was merged, so you'll have to keep clicking run 😆 . If I open another I should be able to see the result and change if necessary.
I saw in one place this was using setup-java but the rest setup-scala. Another opinionated change, but since setup-java also has the default sbt launcher and built-in sbt caching I think it's a better choice to just use that unless it's being included for another reason. This also makes a small change on the sourcegraph upload to use the npm installed src instead of curling it down manually.
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.
This is fantastic! Thank you so much.
jobs: | ||
scip: | ||
runs-on: ubuntu-latest | ||
name: "Upload SCIP" | ||
steps: | ||
- uses: actions/checkout@v2 | ||
- uses: olafurpg/setup-scala@v13 | ||
- uses: actions/setup-java@v3 | ||
- uses: actions/setup-go@v2 | ||
with: | ||
go-version: "1.15.6" |
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.
This might be a remnant from the time when we build src
from source. If CI is green without this step then it's fine to remove it
I saw in one place this was using setup-java but the rest setup-scala.
Another opinionated change, but since setup-java also has the default
sbt launcher and built-in sbt caching I think it's a better choice to
just use that unless it's being included for another reason.
This also makes a small change on the sourcegraph upload to use the npm
installed src instead of curling it down manually.
Test plan
Everything should remain 🟢
Supersedes #454