Skip to content
This repository has been archived by the owner on Jun 15, 2021. It is now read-only.

[WIP] Getting Started Guide #43

Closed
wants to merge 8 commits into from
Closed

[WIP] Getting Started Guide #43

wants to merge 8 commits into from

Conversation

m52go
Copy link
Contributor

@m52go m52go commented May 2, 2018

I probably went a little overboard for what's supposed to be an outline, but there's still plenty to do. I'm aware that the extra work I did is my own liability.

@cbeams
Copy link
Member

cbeams commented May 2, 2018

Looks great at a glance, @m52go, I'll provide more detailed comments later, but in the meantime, I've pushed a couple review commits, please see and study them above.

A few other nits:

  1. Pease read and follow our guidelines around commit comments at Follow the seven rules laid out in "How to Write a Git Commit Message" style#9.
  2. Configure your Git client for proper author metadata (first and last name)
  3. Set yourself up for GPG commit signing
  4. In the future, please submit PRs from a branch other than your master branch, e.g. in this case, getting-started would have been fine. I'll omit the reasons for this here, but can explain more fully later if you like.

Links to and further information about each of these things can be found in the contributor checklist.

Note that you can go back and touch up your existing commits with git rebase --interactive. You don't strictly need to do this, but if you're not already familiar with this hugely powerful Git tool, I'd recommend taking a little quality time to learn it. It allows you to rewrite history, change details about commits, etc. In this case, what you want to do is "reword" the commits above to conform to the style guidelines. Again, not a strict requirement at all, but mentioning it as this is a tool I use every day, and usually find that others are very happy to know about once they've learned it too.

@cbeams cbeams self-requested a review May 2, 2018 13:19
Bernard Labno and others added 8 commits May 3, 2018 08:09
@m52go
Copy link
Contributor Author

m52go commented May 3, 2018

This PR was pretty horrible from a git perspective. I've cleaned everything up...updated old commit messages, set up signing, and made a new branch.

Looks like the only thing I can't do is switch away from master on this PR.

Should I just submit a new PR (for consistency with the rest of the repo), since there's no discussion here yet about this PR content?

@cbeams
Copy link
Member

cbeams commented May 3, 2018

Sure, @m52go, feel free to close this and issue a new pull request from your new branch. Before you do, though, make sure your new branch actually contains only the commits related to this doc. Notice above how there are commits from Bernard and me that have nothing to do with this doc? This is part of the reason that issuing pull requests from master is a bad idea. It ends up creating all kinds of chaos like this (I'll leave aside the details as to why this happens).

The easiest thing to do should be the following:

git checkout master
git fetch origin
git reset --hard origin/master
git checkout getting-started # or whatever your new branch name is
git rebase master
git log --oneline --decorate -10 # the only commits between `master` and `getting-started` should be your 6 commits related to this doc. If not, chat with me in Slack about it
git push --set-upstream m52go getting-started # push your pull request branch up to your remote
# re-issue the pull request from this new branch in the same way you did it the first time

If any of that doesn't work or doesn't make sense, just let me know.

@cbeams
Copy link
Member

cbeams commented May 3, 2018

Also, please reference this pull request in the description of your new pull request, with a quick note as to why you created the new one, just for posterity.

@cbeams
Copy link
Member

cbeams commented May 3, 2018

Also @m52go, when you create your new pull request, please reference #37 in the description. Whenever you're creating a pull request that relates to some already existing issue, it's a good idea to link to it in this way, so that the pull request is discoverable from the original issue (the reference causes a link to the pull request to show up in the original issue's event timeline).

@m52go
Copy link
Contributor Author

m52go commented May 3, 2018

Please refer to #45 for updates.

@m52go m52go closed this May 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants