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: compile using wake 0.18.1 #2584

Merged
merged 1 commit into from
Aug 4, 2020
Merged

CI: compile using wake 0.18.1 #2584

merged 1 commit into from
Aug 4, 2020

Conversation

terpstra
Copy link
Contributor

@terpstra terpstra commented Aug 4, 2020

Type of change: other enhancement
Impact: no functional change
Development Phase: implementation

Stop regressing on wake 0.18 support; compile using 0.18.1 instead of 0.17.2.

@terpstra
Copy link
Contributor Author

terpstra commented Aug 4, 2020

This one turned out to be very easy.

Copy link
Member

@richardxia richardxia left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -34,5 +34,5 @@ echo "Compile Scala"

export WAKE_PATH=$PATH
wake --init .
wake --no-tty -p1 -dv 'compileScalaModule rocketchipScalaModule | getPathResult'
wake -x 'compileScalaModule rocketchipScalaModule | getPathResult'
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you got rid of the --no-tty? I think this affects the carriage return thing for "progress bars", and in CI, when you don't really have a real TTY, it often ends up creating a newline for each carriage return as well.

Copy link
Contributor Author

@terpstra terpstra Aug 4, 2020

Choose a reason for hiding this comment

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

Wake does not need to be told there is no tty. It auto-detects this.
The only reason to use --no-tty is if you actually DO have a tty, but don't want the bars.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see that you're right. I took a quick look at the actual CI console log from this PR and saw some progress bars, but after taking a closer look, I see that they're actually from curl.

@terpstra terpstra merged commit 5613085 into master Aug 4, 2020
@terpstra terpstra deleted the wake-0.18.1 branch August 4, 2020 21:13
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