Skip to content

Update and clarify preview instructions.#41

Merged
mbrukman merged 1 commit intoJanusGraph:masterfrom
mbrukman:update-preview-instructions
May 25, 2018
Merged

Update and clarify preview instructions.#41
mbrukman merged 1 commit intoJanusGraph:masterfrom
mbrukman:update-preview-instructions

Conversation

@mbrukman
Copy link
Copy Markdown
Member

Provide more details and clarity about how to submit changes for this
repo and how to make a preview of the website to enable sharing with
reviewers and other observers.

Comment thread README.md Outdated

```
$ git checkout master
$ git pull upstream master
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming my clone is old one, we should also do git merge upstream/master to make sure local master has latest from the upstream repo.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

git pull upstream master will do exactly that, no?

What I typically run myself is actually:

git pull upstream master --ff-only

which ensures a fast-forward (clean) merge, and it fails if I have any changes on my master added accidentally, which should have gone through a feature branch and merged upstream first, before showing up on my local master, which I try to keep clean and inline with upstream.

@hsaputra
Copy link
Copy Markdown
Member

hsaputra commented Apr 22, 2018 via email

@mbrukman
Copy link
Copy Markdown
Member Author

@hsaputra — what I typically do is:

Bring local master to match upstream:

$ git checkout master
$ git pull upstream master

If I want my GitHub fork's master to be up-to-date (which is really optional, but why not):

$ git push    # automatically pushes local `master` to `origin/master`

If I want to update my feature branch:

$ git checkout my-feature-branch
$ git rebase master
$ git push -f     # update my PR or in-progress branch to have a clean diff

If I merge master into my feature branch, it becomes more complex and has a lot of extra spurious commits, whereas I want it to only have a single commit (or 2 if that's what I'm trying to do specifically).

The rebasing of my feature branch on the updated master is described lower in the README.

Does that address your concern, or were you looking for something else?

@hsaputra
Copy link
Copy Markdown
Member

hsaputra commented Apr 22, 2018 via email

@mbrukman mbrukman force-pushed the update-preview-instructions branch 2 times, most recently from 70f2edd to 7c6114a Compare April 22, 2018 22:06
Comment thread README.md
```
$ git clone git@github.com:[USERNAME]/janusgraph.org.git
$ cd janusgraph.org
$ git remote add upstream git@github.com:JanusGraph/janusgraph.org.git
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Dont you need to do git fetch upstream right after this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Never mind, will be pulled later.

@hsaputra
Copy link
Copy Markdown
Member

@mbrukman Added one last question/ comment on the PR.

Copy link
Copy Markdown
Member

@hsaputra hsaputra left a comment

Choose a reason for hiding this comment

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

LGTM

@mbrukman mbrukman force-pushed the update-preview-instructions branch from 7c6114a to 887b75c Compare April 29, 2018 01:56
Provide more details and clarity about how to submit changes for this
repo and how to make a preview of the website to enable sharing with
reviewers and other observers.

Signed-off-by: Misha Brukman <mbrukman@google.com>
@mbrukman mbrukman force-pushed the update-preview-instructions branch from 887b75c to b92a402 Compare April 29, 2018 01:57
@mbrukman
Copy link
Copy Markdown
Member Author

@hsaputra — I also remembered and added the use of --ff-only flag to ensure that master is a clean copy of upstream and added some documentation about this. Please take a look and let me know what you think. Thanks for the review!

@mbrukman
Copy link
Copy Markdown
Member Author

@hsaputra — is the PR still OK to merge? See my most recent comment above for changes made since your last review. Let me know if you have any concerns before I merge. Thanks!

@hsaputra
Copy link
Copy Markdown
Member

@mbrukman Yeah, LGTM

Sorry I didn't make it clear.

@mbrukman
Copy link
Copy Markdown
Member Author

Thanks, @hsaputra and @amcp, for the reviews!

@mbrukman mbrukman merged commit 367db95 into JanusGraph:master May 25, 2018
@mbrukman mbrukman deleted the update-preview-instructions branch May 25, 2018 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants