-
Notifications
You must be signed in to change notification settings - Fork 1
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
GitHub Contribution Workflow #5
base: main
Are you sure you want to change the base?
GitHub Contribution Workflow #5
Conversation
This addresses Issue elisa-tech#2 - Updated the nav bar structure - Added a link to the GitHub contribution document in the nav bar - Wrote an initial version of the contribution document Signed-off-by: John MacGregor <[email protected]>
…/orientation into githubWorkflow Apparently the merge on the ELISA repo put this branch one commit behind.
This work contributes to Issue elisa-tech#2 - After trying it in GitHub found out that the hashtag syntax works in GitHub, but the syntax used in the document was misleading - changed, hopefully improved - In the Update localRepo section, two code snippets weren't displayed properly Signed-off-by: John MacGregor <[email protected]>
This work contributes to Issue elisa-tech#2 - above - also fixed missing code snippet highlighting in the Create a newFeature branch section Signed-off-by: John MacGregor <[email protected]>
This work contributes to Issue elisa-tech#2 Simplified the workflow to sync the newFeature branch with the projectRepo/master from pulling to newFeature/master and then merging newFeature/master into newFeature to fetching userRepo and rebasing newFeature. Signed-off-by: John MacGregor <[email protected]>
This work contributes to Issue elisa-tech#2 - to call it before creating a new branch - to make it a default switch in the git pull command Signed-off-by: John MacGregor <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments / suggestions from first pass
$ git pull origin master --ff-only | ||
|
||
~~~ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also fetch and base the feature branch directly on origin/master to minimize mess up potential even more as Paul suggested, see elisa-tech/wg-automotive#21
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm..... this one's as clear as mud to me.
Couldn't find Paul's comment in the automotive WG pull request. Maybe you mean his e-mail?
In the Automotive WG description, you've used git checkout
with the --no-track option with origin/master as argument.
- It's not clear to me why git would assume that the tracking branch on origin would be "master" and not the new branch
- Does this have to be done every time the user switches to another branch and tries to switch back?
- The
git branch
andgit checkout
documentation just say that it doesn't set up a new tracking branch configuration on origin, nothing about origin/master
In his e-mail, Paul used git pull
for this action. I'm not so sure why a fetch at the position you've marked is better.
In the "Update localRepo from userRepo" section (in "Create Content"), I've mentioned using git fetch
without providing a command. Fixed this. Please check.
Leaving this open. Maybe we should discuss this with Paul (don't see how to cc him...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops!
@reiterative I didn't read the comments close enough. I really wanted you to look at this... Sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no 'right way' to do this.
When creating a feature branch, I think the following sequence is easiest to remember (apart form the --ff-only
flag, but you can make this the default behaviour)
- Checkout my local copy of master
git checkout master
- Update that local copy from the remote (but only if I can do fast-forward merges)
git pull --ff-only origin master
- Create a new branch from the updated master branch
git checkout -b my_feature_branch
However, this runs the risk that the user will inadvertently omit step 3, make changes in master
, and then end up in trouble when they later pull some changes from origin
. (The trouble is even more likely to happen if they didn't specify --ff-only
, because they may find themselves dealing with a merge conflict when they call git pull
)
The alternative is to avoid switching to master
altogether by following this sequence:
- Fetch all the latest changes from the remote
git fetch origin
- Create a new branch from the remote branch, but don't track the upstream branch
git checkout -b my_feature --no-track origin/master
However this syntax is not very memorable.
It is especially confusing if you don't understand the significance of origin/master
, and what 'track' means in this context. You can read all the gory details here, but the TLDR is:
- When you call
git fetch remote_name
, git creates (or updates) special copies of all the branches inremote_name
within your local repository. (Conventionally, the default remote name isorigin
, but this name has no special significance) - These branches are referred to in your local repository using the name of the remote i.e.
remote_name/branch_name
and are treated differently to local branches. It's safest to think of them as 'bookmarks' or 'references' to the state of the branch in the remote repository. - By default, when you perform a
git checkout
on a remote branch (eithergit checkout remote_name branch_name
orgit checkout remote_name/branch_name
), the new local branch that is created (the tracking branch) is set up with a default relationship with the remote branch (the upstream branch). Using the--no-track
flag when creating a branch viagit checkout -b
omits this default behaviour - This means that the upstream branch is used as the default target for push and pull operations when you have the tracking branch checked out. In other words, if you call
git push
without specifying a remote or a remote branch, then git will push your changes to the upstream branch associated with your current branch.
The reason git fetch
is arguably preferable is that git pull
is actually two operations (fetch and merge) wrapped in one, with some default behaviour that can terrify the newbie. Encouraging new git users to use explicit fetch and merge operations can help them to grok this more quickly. It also allows them to review the changes that they have fetched before deciding whether to merge them into their local branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Answering your questions more directly:
- It's not clear to me why git would assume that the tracking branch on origin would
be "master" and not the new branch
It would assume that because you are explicitly telling it to create the new branch from origin/master
(i.e. the remote copy of master
), and the default behaviour when git checkout -b
is called on a remote branch is to track that remote branch.
- Does this have to be done every time the user switches to another branch and tries
to switch back?
No. The --no-track
is only needed when creating the branch.
- The
git branch
andgit checkout
documentation just say that it doesn't set up a
new tracking branch configuration on origin, nothing about origin/master
origin/master
is a local copy of the remote branch that represents the last-fetched state of that branch
In his e-mail, Paul used
git pull
for this action. I'm not so sure why a fetch at
the position you've marked is better.
See previous comment. It's arguably better to git fetch
because git pull
may result in an inadvertent merge.
This work contributes to Issue elisa-tech#2 Fixed various issues identfied by - Paul Albertella on the 30th of September 2021 - Jochen Kall on the 30th of September 2021 Signed-off-by: John MacGregor <[email protected]>
This work contributes to Issue elisa-tech#2 - typo mastter - Added a link to a general explanation of the fork/clone/pull request model in GitHub docs. - Consistently put the link to the documentation directly behind a command reference - Added a link to an explanation of the DCO Signed-off-by: John MacGregor <[email protected]>
|
||
You can also configure **newFeature** to be the default to be pushed to by using: | ||
* `git push -u origin newFeature`{:.language-shell .highlight} when you first push it, or | ||
* `git branch --set-upstream newFeature origin/newFeature`{:.language-shell .highlight} if you forget |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried it out. Got the following error message:
fatal: the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead.
Git version: 2.33.1
Tried "set-upstream-to". Got the following error message:
fatal: branch 'origin/newFeature' does not exist
OK, mea culpa. i didn't push the branch to origin before creating content. Am now in the conundrum that I'd have to push an incomplete version to userRepo to create the tracking branch...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reiterative Paul can you take a look at the above comment?
I just found the @mention facility and I'm trying it out. Can you confirm that you received a notification? Thx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can confirm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing how approval manifests ^^
|
||
$ git cs -a -m "commit message" | ||
|
||
~~~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section has to be looked at more closely, possibly after a discussion in the weekly sync.
Commits are marked as verified in the PR. See About commit signature verification.
a) Commits which are submitted from a local copy and signed with only name and email have no verification status displayed ==> implicitly not verified and cannot be verified by the maintainer.
b) Developers can set "Vigilant mode" to mark all of their commits as verified. We should probably recommend that they set it. Vigilant mode is in beta, however.
Adding @Jochen-Kall and @reiterative on cc
Work related to Issue #4 (! Numbering changed...)