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

update default exercism assignment path to ~/exercism #97

Merged
merged 4 commits into from
Aug 24, 2014

Conversation

lcowell
Copy link
Contributor

@lcowell lcowell commented Aug 18, 2014

Previously the default path (if none was provided by the user) when logging in defaulted to a folder called exercism in the current working directory. Now the default path is relative to the user's home directory. If the user requests a demo the demo directory will also be relative to the home directory.

Feedback welcome!

Closes #96

Returning a blank string if the user has an incorrectly setup
environment could mean we puts things in unexpected places.
There is no longer any errors to return, so I've remove the error from
the method signature.
This allows us to set a home directory when testing, which makes it
easier to work with tests that depend on its location.
* use config.DefaultAssignmentPath when user input is blank for
directory
@kytrinyx
Copy link
Member

❤️ I'm stoked to see this, and too fried to think straight. I'll get back to you tomorrow.

return os.Getenv("HOME")
// TODO should we fall back to the CWD instead ?
if homeDir == "" {
panic("unable to determine the location of your home directory")
Copy link
Member

Choose a reason for hiding this comment

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

This should be an errors.New() rather than a panic, and we should handle it in from main() my informing the user and doing an os.Exit(1).

@kytrinyx
Copy link
Member

This all looks better than what is there, it's just that as I start thinking about anything in here critically (my code or yours), I come up with things that I dislike, or ideas that I think would be better.

@lcowell
Copy link
Contributor Author

lcowell commented Aug 23, 2014

Thanks for the feedback. I'm out of town until next week. I'll pick this up when I'm back.

@kytrinyx
Copy link
Member

If you're OK with it, I'll pull it in and then tweak it.

@lcowell
Copy link
Contributor Author

lcowell commented Aug 24, 2014

Go for it!
On Aug 23, 2014 5:06 PM, "Katrina Owen" [email protected] wrote:

If you're OK with it, I'll pull it in and then tweak it.

Reply to this email directly or view it on GitHub
#97 (comment).

kytrinyx added a commit that referenced this pull request Aug 24, 2014
update default exercism assignment path to ~/exercism
@kytrinyx kytrinyx merged commit 34d9150 into exercism:master Aug 24, 2014
lcowell pushed a commit to lcowell/cli that referenced this pull request Jan 25, 2015
update default exercism assignment path to ~/exercism
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.

Make the default directory ~/exercism, not the current directory
2 participants