potiuk commented on issue #6758: [AIRFLOW-6195] Fixed TaskInstance attrs not correct on UI URL: https://github.com/apache/airflow/pull/6758#issuecomment-564459089 Some comments (this is the workflow I use). > To keep my local master up to date: > git checkout master > git fetch upstream > git merge upstream/master master I very rarely keep local master sync-only when I need to create a new branch locally. If you need it, it should be enough to just do `git pull --ff-only`. We (almost) never push --force to master. So any pull should be fast-forward only. If it is not, the --ff-only should cause it to fail (but this almost never happen - I did it once and regret it). > Update my fork's master: > git push -f origin > To keep my feature branch up to date: > git checkout > git rebase -i master > (squash into 1 commit per commit guidelines) > git push -f origin What I usually do to keep my feature branch is I rebase 'onto' the upstream/master branch. It needs a bit more consciousness to know where your commit started. I look at git log and see the last commit before those that I want to rebase and I do `git rebase <hash_of_the_first_commit_I_do_not_want_to_rebase> --onto upstream/master`. It's a little more involved as you need to look up where you branched-off but it's usually pretty obvious. > Here is where I think I'm going wrong. > I had thought that exactly 1 commit was needed. During the review 1 commit is not needed - especially if you work incrementally and add fixups after review. This makes it easier to review as you apply fixes. So what I usually do is: `git commit --fixup HEAD` or `git commit --fixup <hash_of_the_commit_I_want_to_update>`. Then at some point of time it is enough to do `git rebase -j <hash-of-some-older-commit> --autosquash` and those fixups will be automatically joined with the commits they were fixing. But for review purpose I keep those fixup for some time so that reviewers can look only at the changes I made as result of the review. > So if I understand you correctly, the changes made because of code review should not be squashed into the first commit, i.e there should be 2+ commits as a result of the review process. 1 original, +1 for each review. > Then you (committers/maintainers) can squash them all again down to 1 commit when the PR gets merged. > > Is that roughly correct? Correct. Or even better you can rebase + squash just before merge - which is a good practice as there might be for example new pre-commit checks added independently. > > For someone new to re-basing (and contributing in general) I did followed the contrib guide but it was a little unclear to me (also I once force pushed and lost changes to one of my first PRs :) . Anyway I was planning to clarify and add these detailed steps to the contrib guide to make it clearer for others, so really appreciate your feedback here :) I think we do not have a "standard" workflow - we leave people quite a bit of freedom how they work. But maybe we should. Mine is just an example and is a bit involved and 'geeky'. Regarding losing changes - it's really hard to lose anything in git. Even if you force-push there is always `git reflog` to the rescue and you can checkout any commit (by hash) you had in your local .git until it's garbage collected (which by default is 30 days or so). Learning reflog taught me to be a little more courageous when playing with git.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
