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

ci: add Chisel Blink, use Action setup-scala #384

Merged
merged 4 commits into from
Nov 20, 2020
Merged

Conversation

ekiwi
Copy link
Contributor

@ekiwi ekiwi commented Nov 20, 2020

This solves one half of issue #383

Copy link
Collaborator

@umarcor umarcor left a comment

Choose a reason for hiding this comment

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

What's the motivation for using two different Actions to setup Java/Scala? It seems that both of them support all three platforms. Should there be a limitation in actions/setup-java that requires the other one, I'd suggest using it for all platforms and adding a comment explaining why that is required.

At the same time, the toolchain can be installed locally by using the same script as in CI. The only requirement is Python. What would be the minimal requirements for someone to try this example locally? That is, what's the alternative to setup-java|setup-scala? Note that I don't expect it to be explained here, but to add some reference to Scala/Chisel docs in an upcoming PR.

@ekiwi
Copy link
Contributor Author

ekiwi commented Nov 20, 2020

What's the motivation for using two different Actions to setup Java/Scala? It seems that both of them support all three platforms. Should there be a limitation in actions/setup-java that requires the other one, I'd suggest using it for all platforms and adding a comment explaining why that is required.

I changed the CI to use setup-scala for all OS.

Do you want me to squash my commits?

At the same time, the toolchain can be installed locally by using the same script as in CI. The only requirement is Python. What would be the minimal requirements for someone to try this example locally? That is, what's the alternative to setup-java|setup-scala? Note that I don't expect it to be explained here, but to add some reference to Scala/Chisel docs in an upcoming PR.

I will keep that in mind. Essentially you just need a recent version of Java (like OpenJDK 11) and the sbt build tool which will in turn download the correct Scala compiler version etc. once you run the build.

@umarcor
Copy link
Collaborator

umarcor commented Nov 20, 2020

I changed the CI to use setup-scala for all OS.

What was the reason to choose that instead of the other?

Do you want me to squash my commits?

No need, GitHub allows to do it while merging.

@ekiwi
Copy link
Contributor Author

ekiwi commented Nov 20, 2020

What was the reason to choose that instead of the other?

On Windows and Ubuntu sbt (the scala build tool) seems to be installed, but not on MacOS. setup-scala makes sure that sbt is also installed.

@umarcor umarcor changed the title Add Chisel Blink to CI ci: add Chisel Blink, use Action setup-scala Nov 20, 2020
@umarcor umarcor merged commit 7bfdd33 into im-tomu:master Nov 20, 2020
umarcor added a commit to umarcor/fomu-toolchain that referenced this pull request Apr 26, 2021
xobs added a commit to im-tomu/fomu-toolchain that referenced this pull request Apr 27, 2021
ci: install Scala on MacOS using Action olafurpg/setup-scala (im-tomu/fomu-workshop#384)
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