Add developer's docs on Git workflows#1072
Conversation
wgu-taylor-payne
left a comment
wgu-taylor-payne
left a comment
There was a problem hiding this comment.
This added documentation is great and will serve as a nice reference! I've added some comments of things that could be cleaned up or improved upon.
|
|
||
| Making a pull request is very similar to the simple one-remote workflow: | ||
|
|
||
| #. Create a branch locally: ``git switch -c user/description``. |
There was a problem hiding this comment.
I'm not familiar - should branch names be in this form when working on Open edX repos?
There was a problem hiding this comment.
When pushing branches to the upstream, yes.
When pushing branches to personal/org forks, that's totally up to you/your org.
There was a problem hiding this comment.
FWIW, I personally default to using my own forks for everything, regardless of whether I have access to push to the upstream repo. The only time I push branches to the upstream repo are when CI permissions demand it, like what happens with edx-val and Pactflow (which is very rare).
There was a problem hiding this comment.
I have noticed many maintainers pushing to their maintained repositories so I still think this recommendation makes sense.
There was a problem hiding this comment.
I agree, the username prefixing never hurts, so I think it's good to leave this recommendation as-is. Developers can of course push whatever branch names they like to their personal forks, but we don't need to complicate this guidance by spelling that out.
sarina
force-pushed
the
devguide/git-resources
branch
from
244e342 to
1fe7a30
Compare
| * Before submitting your PR for review, perform an `interactive rebase`_, and | ||
| squash away any commits which will not be useful to other developers. | ||
| * During your PR review, as you edit your code in response to feedback, name | ||
| your new commits using the `squash:` prefix. This will indicate to the |
There was a problem hiding this comment.
Would squash! and fixup! messages be more convenient for those doing the merge/rebase into main?
There was a problem hiding this comment.
! means breaking changes, so I don't think that's appropriate.
There was a problem hiding this comment.
If you use the --squash or --fixup flags while committing, git will create a commit with a message that starts with squash! or fixup!. Any commits that start with those will automatically be squashed when you use the --autosquash flag during a rebase.
I actually just learned about these while reading OEP-51. It mentions these types of commits under the Discussion section and Tooling section.
There was a problem hiding this comment.
Good point Taylor, I agree that we should encourage the use of built-in git message formats like squash! and fixup!, even though they are similar to conventional commits' notation for breaking changes.
Small point of information: The syntax for these is squash! <message>, not squash!: message, so it is subtly different than a conventional commit breaking change message.
These commits should never show up in main or master (because they'll be squashed away) so I'm not too concerned about the confusion.
There was a problem hiding this comment.
Will using squash! or fixup! on a WIP PR cause the commitlint to fail?
If not, would someone want to suggest some text to put here?
There was a problem hiding this comment.
I opened a suggestion thread over here: #1072 (comment)
|
@kdmccormick @wgu-taylor-payne any other feedback? |
|
@kdmccormick committed you change, let's make sure it renders correctly then you can approve/merge. One note is that it doesn't seem possible to make |
kdmccormick
left a comment
kdmccormick
left a comment
There was a problem hiding this comment.
Whoops, my suggestion was markdown instead of rST, thanks for fixing that.
LGTM!
sarina
force-pushed
the
devguide/git-resources
branch
from
fb5e2a2 to
dbafde9
Compare
4 participants
https://docsopenedxorg--1072.org.readthedocs.build/en/1072/developers/references/developer_guide/process/using-git.html#keeping-your-fork-up-to-date
Migrated this doc: https://openedx.atlassian.net/wiki/spaces/AC/pages/3458170881/Working+with+Personal+Forks