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.

Sign up for GitHub

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

Jump to bottom

Search up the tree for the config file. #23

Merged
merged 2 commits into from
Oct 17, 2017
Merged

Search up the tree for the config file. #23

merged 2 commits into from
Oct 17, 2017

Conversation

Asuza
Copy link
Contributor

@Asuza Asuza commented Oct 14, 2017

Fixes #4.

sapegin
sapegin requested changes Oct 14, 2017
View reviewed changes
Copy link
Owner

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Thanks!

bin/shipit Outdated
CONFIG=./.shipit
CONFIG_DIR=`pwd`
CONFIG_NAME=.shipit
CONFIG="$CONFIG_DIR$CONFIG_NAME"
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like you’re overwriting this value anyway, could be empty by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's there so if the config file is never found, the existing error message still shows up as expected. I'm pretty sure that conditional wasn't working when I left this blank.

The value only gets overwritten if the config exists somewhere in the tree.

Copy link
Owner

@sapegin sapegin Oct 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I believe it will work like this: test -f "$CONFIG", with quotes.

@Asuza
Copy link
Contributor Author

Asuza commented Oct 17, 2017

@sapegin, I updated this PR as requested.

@Asuza
Copy link
Contributor Author

Asuza commented Oct 17, 2017 •
edited
Loading

Do I need to bump the version after you're happy with my PRs? Or do you need to do that?

@sapegin sapegin merged commit 732d4bc into sapegin:master Oct 17, 2017
@sapegin
Copy link
Owner

sapegin commented Oct 17, 2017

Thanks! I’ll update the version myself.

@sapegin
Copy link
Owner

sapegin commented Oct 17, 2017

Out in 0.2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sapegin sapegin sapegin requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Asuza @sapegin