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

Check root #163

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Check root #163

wants to merge 4 commits into from

Conversation

gforsyth
Copy link
Member

@gforsyth gforsyth commented Mar 8, 2017

If we are in a git repo but not at the root of it, print error and allow
user to --force doctr to run anyway.

If not in a git repo at all, raise a RuntimeError

Should address #162

gforsyth added 3 commits March 8, 2017 17:29
If we are in a git repo but not at the root of it, print error and allow
user to ``--force`` doctr to run anyway.

If not in a git repo at all, raise a RuntimeError
Check if current working directory is root dir of git repo.
Return ``False`` if it isn't.
"""
repo = subprocess.run(['git', 'rev-parse', '--show-toplevel'],
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just check if the current directory has a .git directory?

Copy link
Member

Choose a reason for hiding this comment

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

That wouldn't check if we are in a git repo at all, but I don't see the point have having the distinct error messages in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

My thought was that someone might want to run doctr in a subdirectory for some specific reason? I can't think of what that would be, but that was the thought.

Copy link
Member

Choose a reason for hiding this comment

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

Well it doesn't have to be at the root of the repo, unless I'm mistaken, as long as the paths to the key and the other arguments are given to it, relative to the current directory. It does need to be in the repo, as it calls git.

So I actually think it should only error when it is not in the git repo (it will error anyway, eventually, because git will complain as soon as doctr calls it), and warn, but not error when it isn't, just so it is easier for people to debug if they accidentally are cd'd to the wrong directory and are trying to figure out why doctr failed.

Copy link
Member

Choose a reason for hiding this comment

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

OTOH, maybe weird stuff will happen when checking out gh-pages if we aren't in the root (unless the directory we are in isn't tracked, or is in both branches).

@asmeurer
Copy link
Member

asmeurer commented Mar 8, 2017

The tests are failing from that race condition again.

@gforsyth gforsyth mentioned this pull request Mar 14, 2017
@ylemkimon
Copy link
Contributor

How about checking current working directory is TRAVIS_BUILD_DIR?

@asmeurer
Copy link
Member

This is for doctr configure, which is run on the local compute, not Travis.

@ylemkimon
Copy link
Contributor

ylemkimon commented Nov 20, 2017

@asmeurer I thought this was for #274. I think checking current working directory is TRAVIS_BUILD_DIR may address that issue.

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.

3 participants