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

Sphinx doc #31

Merged
merged 19 commits into from May 6, 2015
Merged

Sphinx doc #31

merged 19 commits into from May 6, 2015

Conversation

weaverba137
Copy link
Member

Fixes #29.

I want to get this pull request in before my branch diverges too far. Rebases suck.

@weaverba137
Copy link
Member Author

PS, I only started this branch yesterday, & there were already ~8 commits to master before I tried to create the PR, hence the need to rebase.

Rebasing really sucks. Who is committing directly to the master branch? This makes it difficult to work on a branch. We may want to consider doing all development as PRs in order to keep master stable.

@sbailey
Copy link
Contributor

sbailey commented May 6, 2015

Wow, lots of files changed, though it looks like mostly to cleanup the docs, plus a few string formatting updates and "from future" tweaks. I will merge it in a moment. It looks like you removed unnecessary trailing whitespace. I'm not sure if this was on purpose or whether your editor did it for your automatically, but a few comments on that:

  • we should avoid trailing whitespace in the first place (though I admit my editor tends to leave them behind as a result of its auto-indent algorithm).
  • we should avoid removing trailing whitespace unless we are actually editing that piece of code as well, since it could cause unnecessary fake merge conflicts
  • ok in this case as a code cleanup pass

sbailey added a commit that referenced this pull request May 6, 2015
Sphinx doc updates; lots of docstrings and comments and trailing whitespace were touched, so everyone should git pull now before editing further.
@sbailey sbailey merged commit 4d609b3 into desihub:master May 6, 2015
@weaverba137 weaverba137 deleted the sphinx-doc branch May 6, 2015 16:47
@sbailey
Copy link
Contributor

sbailey commented May 6, 2015

I'll admit I've been committing directly to master for minor updates. Even if those had gone via branches and pull requests, wouldn't the same "merge from master" issue have occurred? I think this works to bring in the changes prior to pushing a feature branch:

git checkout master
git pull
git checkout my_feature_branch
git merge --no-ff master
#- proceed with edits, adds, commits, pushing, pull requests etc.

I don't think that generates any rebase/merge headaches that wouldn't have otherwise been encountered if the commits had gone in via a branch and pull request that was merged into master. But correct me if I'm wrong.

The --no-ff (no fastforward) isn't strictly necessary, though it preserves the full commit history which I tend to like.

@weaverba137
Copy link
Member Author

OK, I'll try that next time.

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.

get docs + docstrings working for sphinx
2 participants