Page MenuHomePhabricator

[llvm][docs] Add step by step git to GettingStarted
ClosedPublic

Authored by zbrid on Sun, May 17, 10:15 AM.

Details

Summary

Due to deleting the git llvm script, folks were asking for better documentation
about how to use git in order to commit to the Github repo. I added some step
by step git commands to make the usage clearer.

Context link: http://lists.llvm.org/pipermail/llvm-dev/2020-May/141640.html

Diff Detail

Event Timeline

zbrid created this revision.Sun, May 17, 10:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptSun, May 17, 10:15 AM
zbrid updated this revision to Diff 264504.Sun, May 17, 10:30 AM

Add running tests to the workflow

mehdi_amini added inline comments.
llvm/docs/GettingStarted.rst
511

This isn't super useful I believe, this only shows for example something like:

$ git push gh HEAD:master --dry-run
To github.com:llvm/llvm-project.git
   ffc6e593d271..e88c68267154  HEAD -> master

What about instead:

$ git log origin/master...HEAD --oneline

which shows the list of commits about to be pushed?

zbrid updated this revision to Diff 264654.Mon, May 18, 9:12 AM
zbrid marked an inline comment as done.

Change the dry-run command to mehdi's suggestion

zbrid added a comment.Mon, May 18, 9:12 AM

Good idea. Done.

spatel added inline comments.Mon, May 18, 1:09 PM
llvm/docs/GettingStarted.rst
511

Thanks - that's basically the feature of 'git llvm push' that was most useful to me.
It did some variation of this:

$ git llvm push
`git fetch https://github.com/llvm/llvm-project.git master` printed to stderr:
From https://github.com/llvm/llvm-project
 * branch                    master     -> FETCH_HEAD
Pushing 2 commits:
  f29a9387ace [x86] add tests for disguised horizontal ops; NFC
  f40b5065511 [x86] add tests for heroic horizontal ops; NFC
Are you sure you want to create 2 commits? (y/N): n

That 'Are you sure...' pause is what kept me out of trouble, so I'd probably just end up creating a weak version of the old script again.

zbrid marked an inline comment as done.Mon, May 18, 5:04 PM

@spatel - I updated the documentation here to use the command mehdi suggested (that you said was good in a follow up comment). The rest of it look good too?

spatel accepted this revision.Tue, May 19, 5:02 AM

@spatel - I updated the documentation here to use the command mehdi suggested (that you said was good in a follow up comment). The rest of it look good too?

Sure; I don't know if there's more that could be added, but this LGTM.

This revision is now accepted and ready to land.Tue, May 19, 5:02 AM
mehdi_amini accepted this revision.Tue, May 19, 9:04 AM
This revision was automatically updated to reflect the committed changes.