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

Add ability to pass crystal --error-trace to lucky watch task. #957

Merged
merged 3 commits into from
Jan 3, 2020

Conversation

jwoertink
Copy link
Member

Purpose

Fixes #947

Description

Allows you to pass crystal's --error-trace flag in to lucky watch command. When you run lucky dev, and encounter a compiler error, you'll be given the full trace each time.

Checklist

  • - An issue already exists detailing the issue/or feature request that this PR fixes
  • - All specs are formatted with crystal tool format spec src
  • - Inline documentation has been added and/or updated
  • - Lucky builds on docker with ./script/setup
  • - All builds and specs pass on docker with ./script/test

Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

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

This looks good. My only thought here is that we should probqbly see if the Crystal team could add a SHOW_FULL_CRYSTAL_ERROR_TRACE ENV var that can be set to 1 or true so that it people can set it in their shell or before a command and not worry about it passing the flag. This can be helpful to any command line tool written in Crystal

@paulcsmith
Copy link
Member

Added an issue to Crystal here: crystal-lang/crystal#8374

@jwoertink
Copy link
Member Author

I've added a little todo note for us to revisit if that crystal issue ever becomes a thing. I think getting this merged in for now will be good just so people can take advantage of the full errors. I'll merge once these builds finish and pass.

@paulcsmith
Copy link
Member

Agreed! Thanks @jwoertink

@jwoertink jwoertink merged commit a60ac8c into master Jan 3, 2020
@jwoertink jwoertink deleted the issues/947 branch January 3, 2020 18:14
@bdtomlin
Copy link
Contributor

bdtomlin commented Jan 3, 2020

@jwoertink, thanks for this. Super helpful!

@Napolskih
Copy link

image image image

@jwoertink not fixed!

@jwoertink
Copy link
Member Author

@Napolskih You have to make sure that your lucky -v is on at least 1.0.0 or later. Since 1.1.0 is the latest as of right now, you should upgrade to that.

image

@Napolskih
Copy link

image

@jwoertink
Copy link
Member Author

What version is listed in your shard.yml file?

@Napolskih
Copy link

Thanks I got it.

Lucky_jumpstart needs to be removed from the documentation or updated.

image

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.

Add --error-trace to lucky watch / lucky dev
4 participants