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

Remove bin/crystal usage for init tool specs #5520

Merged
merged 5 commits into from Jan 5, 2018

Conversation

bew
Copy link
Contributor

@bew bew commented Jan 3, 2018

It feels ugly to pass stderr all over the place, but hey.. it works!

I think that the routines getting the correct fields from the given args being in the class is not correct, we should probably build a intermediate object (kind of factory which takes arguments and 'parse' them) that will create & return the InitProject object then execute it or sth like that.. Absolutely not sure about this..

Maybe we should just merge this to remove bin/crystal direct usage, and make another PR to refactor the init tool? (I think I would prefer that)

WDYT?

@asterite
Copy link
Member

asterite commented Jan 3, 2018

To avoid passing stderr everywhere, make Init a class, store it in an instance variable, and change class methods to instance methods.

@bew bew force-pushed the spec-init-programatically branch from e750686 to 2b48076 Compare January 3, 2018 20:43
@bew
Copy link
Contributor Author

bew commented Jan 3, 2018

@asterite the specs fails weirdly because there are early exits in the init tool, so when the spec check a specific error, it exits after printing a little msg on stderr. (no idea how I got the specs to pass once on my computer...)

I'm going in another way, using exceptions instead of STDERR.puts msg; exit 1, it should be better!

@RX14
Copy link
Contributor

RX14 commented Jan 4, 2018

circleci seems to be flakey downloading things from homebrew, i've had to restart a number of builds.

Copy link
Member

@asterite asterite left a comment

Choose a reason for hiding this comment

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

I don't think we need two exception types here. Even just using plain Exception might be good, but just Error would be fine.

@bew
Copy link
Contributor Author

bew commented Jan 5, 2018

@asterite I kept Error exception class only, should be GTG I think!

@RX14 RX14 added this to the Next milestone Jan 5, 2018
@RX14 RX14 merged commit 5bdd279 into crystal-lang:master Jan 5, 2018
@bew bew deleted the spec-init-programatically branch January 5, 2018 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants