On 2020-04-19 23:53, [email protected] wrote: > Hello John, > > First, you need to know that I'm not a great lover of gerrit (and > Jenkins), as these legacy systems are bringing a lot of barriers. > But, LF IT seems to think they're more interesting than gitlab or > github/github actions so we have (unfortunately) to live with them. > > Before working for ONAP, I did too use the "git flow" paradigm > (https://buddy.works/blog/5-types-of-git-workflows#gitflow). > > So, Gerrit doesn't work with git flow as we are not allowed to create > branches / forks + merge request but rather we have to send > "reviews". > > A review is actually one (and only one) commit where all your changes > are (think the stash of all your commits from a branch). So, instead > of having one branch with several commits that will eventually (most > of the time, for better readability) be stashed, you directly work on > the "final" commit. > > The main difference are: * you don't have a "develop" branch but I'm > not sure at the end it's a good practice * releases are not tag but > branch (and tags), again this is a different view but it's not that > bad
There really seems to be a disconnect here on how Gerrit operates. Yes, Gerrit operates on a per patch workflow, but patches can be grouped together in virtual brances. These branches in Gerrit parlance are a topic. If you're using the git-review plugin then when you do: git review It by default pushes up _all_ the changes on the local branch as a topic (unless the branch you're working on is a branch that exists remotely already). So, to get the flow of a long lived feature branch it works something like this (yes I actually did all this against a live Gerrit repo): --[cut example long lived feature]-- # assume we haven't clone yet # the example project we'll use a sandbox project on the LF Gerrit # https://gerrit.linuxfoundation.org/infra/admin/repos/sandbox git clone ssh://gerrit.linuxfoundation.org/sandbox.git cd sandbox # setup git-review and get the commit-msg hook git review -s # our new feature is called castle, because we're in a sandbox :D # we're going to do several patches with some re-arranging _after_ # the reviews have started so you can see how this really works # # I'll also add in an unrelated change to master before we submit # the topic so you can see how this really ends up working git checkout -b castle # We already have an example.txt file in the repo, for our first # rather silly change, we're just going to add some more files, but # not have any sort of content touch example{2,3,4}.txt git add example*.txt # and now a really bad commit message. Please see # https://chris.beams.io/posts/git-commit/ # and https://fatbusinessman.com/2019/my-favourite-git-commit # for best practices around commit messages. Also note, the LF Gerrit, # while it _is_ connected to a Jira system, does _not_ require Issue-id # footers like the ONAP Gerrit does. As such, I'm going to ignore those. git commit -sm 'Add some empty example files' # now I'm going to push this review up to get things prepped # I'm also going to switch it to a WIP so that it is not mergeable git review # for change # https://gerrit.linuxfoundation.org/infra/c/sandbox/+/63717 # I select the triple dot menu -> Mark work in progress # on to change two we're going to make some changes to files echo 'More example text' > example2.txt echo 'Lorem ipsum' > example3.txt echo 'dolor sit amet, consectetur adipiscing elit.' > example4.txt # and another commit git commit -asm 'Add some lorem ipsum' # I'm going to hold off on sending up the review for now. # git-review does the right thing if we send up several patches at the # same time # change three echo 'Quisque tincidunt nibh vitae molestie ultrices.' >> example2.txt git commit -asm 'More lorem ipsum' # send up the reviews, I get asked about 3 outsanding changes and I say # yes it ends up only pushing up 2 new changes # https://gerrit.linuxfoundation.org/infra/c/sandbox/+/63718 # https://gerrit.linuxfoundation.org/infra/c/sandbox/+/63719 # We can now see the virtual feature branch in action in Gerrit by using # this query: # https://gerrit.linuxfoundation.org/infra/q/topic:castle # And now one more change in our "castle" feature # this one completely rewrites example2 on purpose for this echo 'Nulla facilisi.' > example2.txt git commit -asm 'Update example2 with new lorem ipsum' # and review git review # https://gerrit.linuxfoundation.org/infra/c/sandbox/+/63720 # and now to prepare for my silliness in making things hard git checkout master && git pull && git checkout -b wave # this puts me right back at the beginning of all of this as nothing # has been merged yet. I'm going to introduce a merge conflict echo 'My wave destroys your castle' > example2.txt git add example2.txt git commit -sm 'Cause merge conflict!' git review # https://gerrit.linuxfoundation.org/infra/c/sandbox/+/63721 # both topics: # https://gerrit.linuxfoundation.org/infra/q/topic:castle+OR+topic:wave # I'm going to go ahead and get our wave merged in # at this point Gerrit the above dual topic search shows me the # following information (rendered in bad ascii) --[cut bad ascii]-- 63721 Cause merge conflict! Merged... 63720 Update example2 with new lorem ipsum Merge Conflict 63719 More lorem ipsum Merge Conflict 63718 Add some lorem ipsum Merge Conflict 63717 Add some empty files WIP --[/cut bad ascii]-- # 63717 doesn't show Merge Conflict because it's currently marked # WIP. If I start the review on that change it will also show # Merge Conflict. I'll go ahead and start the review now so I can # play with things # First, my reviewer (myself) complains about a silly commit # adding empty files instead of just adding the text of the files # directly. I get a -1 on my change. # Let's see how we can fix just _that_ and still keep our merge conflict # for now # I'm _not_ going to do what I normally do and pull the latest master # before starting new work, this is so that I can continue being # based on parent commit 622e818b19b0367589316dbef872107011c3ce28 # I'm going to do some screwy bits now playing with git rebase on # my local castle branch git checkout castle git rebase -i HEAD~4 # I'm going to leave all the lines the same _except_ for switching # pick to s on d5d19da # This lets me merge the two changes and modify the commit messages # I need to do this because I want to keep change 63717 as the # live change and basically abandon 63718 # To do this I need to get rid of the Change-Id line that comes # from commit d5d19da. Basically I'm going to rewrite the commit to: --[cut]-- Add some lorem ipsum Change-Id: Id8dc98ce08be53e7485ff70bb4e7b6b19699d6f5 Signed-off-by: Andrew Grimberg <[email protected]> --[/cut]-- # I don't want git review to try to rebase automatically for me # I know there is a merge conflict and I'm doing this as an example git review -R # Ok, we've still got 4 changes open, one of which is off to the side # now. We can abandon 63718 # Going into it what I see is the following in the the relation chain # on the right --[cut]-- Update example2 with new lorem ipsum (Not Current) More lorem ipsum (Not Current) -> Add some lorem ipsum # this is the change I'm actually looking at Add some empty example files (Not Current) --[cut]-- # This tells me a few things: # 1) all of the changes that are part if _this_ patch series # are now at different versions # 2) I as the author know that this change can actually be abandoned # As such, I'm going to whack that 'Abandon' link and give a reason # Ok, so, at this point I as the author of the change need to fix # my merge conflicts, how am I going to do that? # First, let's get my local master up to date git checkout master && git pull # I think the best way forward is to get the base change fixed # rather than mess with my current local branch, I'm going to use # the change download feature of git-review to play with just that one # change first git review -d 63717,2 # Now I'm going to rebase my current change onto master and # fix my merge conflicts which means fixing example2.txt # I'm just going to take my lorem ipsum change so I make the needed # fixes to example2.txt and then git add exampl2.txt git rebase --continue # Now to pull in the rest of my patch series those commits are: # 76f381feb0f59554087d4982a74a24508a52048a and # fa02df2af868bd7012faf67d6c91a2facc8e56c4 on my castle branch git cherry-pick 76f381feb0f59554087d4982a74a24508a52048a git cherry-pick fa02df2af868bd7012faf67d6c91a2facc8e56c4 # and push them back up and make sure I'm still on the same topic git review -t castle # and just to add in something that we're going to merge safely around # standard practice when starting a new "feature" git checkout master && git pull git checkout -b safe_merge echo 'Nam nec dapibus eros, non tincidunt eros.' > example5.txt git add example5.txt git commit -sm 'Example 5 of lorem ipsum' git review # https://gerrit.linuxfoundation.org/infra/c/sandbox/+/63723 # and get that merged # New topic search shows me: # https://gerrit.linuxfoundation.org/infra/q/topic:castle+OR+topic:wave+OR+topic:safe_merge --[cut]-- 63723 Example 5 of lorem ipsum Merged 63720 Update example2 with new lorem ipsum -- 63719 More lorum ipsum -- 63717 Add some lorem ipsum -- 63718 Add some lorem ipsum Abandoned 63721 Cause merge conflict! Merged --[/cut]-- # So at this point I'm just going to go for a merge set. # I'm going to go ahead and give code review at max for all the changes # but not submit till I'm on the tip of the set # Once the full set has passed all votes the tip (and in between changes # too will have a 'Submit including parents' link for committers # The base of the change set will just have 'Submit' # I go ahead and hit that on the tip change and the whole feature branch # merges in at the same time # At this point the master branch is now up to date with the feature # branch, looking over the graph I didn't quite get what I was hoping # for, a non-fast-forward merge, but the graph does look somewhat # interesting. Had I managed to get a non-fast-forward, but # automergeable change in there Gerrit would have created a merge object # at the merge point. --[/cut example long lived feature]-- The benefit of this method is that you _don't_ have to create new CI jobs pointing at another branch to run your tests. Technically you're working directly against master the whole time and therefore leveraging the jobs already setup. You still get your lightweight feature branches, _and_ you don't actually have to have those branches created in the central repository thereby cluttering our namespace of the officially released code nor getting mirrored to our GitHub mirrors. -Andy- > This gives pros and cons: pros: * only one commit, making git history > simple * no commit with message like "typo fix", "oops", "reverting > again" and so on > > cons: * To work with several features in parallel, it's up to you to > use the workflow locally you want. I personally create a branch per > feature but any solution should work * if your push by mistake a > second commit, it will become a new merge request (with relationship > but still) > > But, at the end, it's very similar to Pull Request / Merge Request we > can have on github/gitlab > > Regards, Sylvain > > ________________________________________ De : FRANEY, JOHN J > [[email protected]] Envoyé : dimanche 19 avril 2020 19:33 À : > [email protected]; [email protected]; > DESBUREAUX Sylvain TGI/OLN; [email protected] Objet : RE: > [onap-discuss] onap git flow convention? > > Andrew, and anyone, > > Can anyone more deeply explain why ONAP is striving to avoid > branching/merging in its git flow? > > Andrew, Can you detail a specific limitation that will help me come > to terms with this policy? I hope you can break this down because I > cannot get my head around it: "It's something that can be enabled > but it breaks the workflow that Gerrit is striving for in that a > single change, or even patches (series of different changes that > depend upon each other) are well reviewed and if CI is hooked up (as > is the case with ONAP) that _no_ single change when merged should > cause a break (thought without running a CI system like Zuul we can't > guarantee that any particular merge order will _not_ break)." > > > > > Without branching/merging, I think the internal AT&T team for AAF > would have to create some sort of shadow project. Is that what > other teams do? By shadow project, I mean that a team would run its > own clone, internally or git hub, where branching/merging is > permitted. Then, dump them all up to ONAP's repo when master is open > to contribution. This requires more effort (mostly housekeeping) and > adds risk. How has other teams dealt with these? > > Shadow build environment: Does the internal team need to replicate > ONAP's CI/CD Jenkins jobs? Probably. If not, ONAP specific scans > would not occur until the internal changes are dumped up to ONAP's > repo. > > Diverging release roadmap: If AT&T team wants to build a release use > between ONAP releases, it would be for internal use only; the team > would have to rename the java artifacts to prevent a name clash with > ONAP artifacts. > > Shadow issue tracking: The AT&T team would track these changes in > corporate internal jira, not ONAP's, and maintains some convention to > map the issue keys. > > Shadow static scans: The internal team would have to run a different > CI/CD environment; the team would not be able to take advantage of > automatic scans built into ONAP's job builder. > > Divergence due to refactoring: Refactoring to remove technical debt > can be more risky because these changes would not immediately merge > to ONAP's repo. > > Diverging interests: Features and fixes driven solely by > stakeholders within AT&T may not be contributed to ONAP at all. > > > Thanks for any help. I'm hoping to fit into ONAP's process, but I'm > also groping to understand it. > > John > > > -----Original Message----- From: [email protected] > <[email protected]> On Behalf Of Andrew Grimberg Sent: > Friday, April 17, 2020 4:48 PM To: FRANEY, JOHN J <[email protected]>; > [email protected]; [email protected]; > [email protected] Subject: Re: [onap-discuss] onap git flow > convention? > > On 2020-04-17 11:28, FRANEY, JOHN J wrote: >> Thanks Andrew, >> >> I doubt that I am understanding correctly. Git was designed >> specifically around quick efficient frequent merging. Really. >> Easy merging is the primary goal of git's design. >> >> And gerrit disallows merges? >> >> sad. ☹ (Isn't distributed software development difficult >> enough?) > > Our configuration (and the default configuration of Gerrit) disallow > merge commit changes to be proposed. It's something that can be > enabled but it breaks the workflow that Gerrit is striving for in > that a single change, or even patches (series of different changes > that depend upon each other) are well reviewed and if CI is hooked up > (as is the case with ONAP) that _no_ single change when merged should > cause a break (thought without running a CI system like Zuul we can't > guarantee that any particular merge order will _not_ break). > > So, what I'm saying is this: > > You, me, anyone, cannot propose a change that consists of a merge of > changes. > > What you can propose is a single change, or series of changes that > depend on each other that when all validation has passed and a > committer 'submits' the change may either be a fast-forward merge, or > a standard merge. > > Basically, we disallow you to push something that is specifically a > merge commit object. > > -Andy- > > -- Andrew J Grimberg Manager Release Engineering The Linux > Foundation > > > > > > _________________________________________________________________________________________________________________________ > > Ce message et ses pieces jointes peuvent contenir des informations > confidentielles ou privilegiees et ne doivent donc pas etre diffuses, > exploites ou copies sans autorisation. Si vous avez recu ce message > par erreur, veuillez le signaler a l'expediteur et le detruire ainsi > que les pieces jointes. Les messages electroniques etant susceptibles > d'alteration, Orange decline toute responsabilite si ce message a ete > altere, deforme ou falsifie. Merci. > > This message and its attachments may contain confidential or > privileged information that may be protected by law; they should not > be distributed, used or copied without authorisation. If you have > received this email in error, please notify the sender and delete > this message and its attachments. As emails may be altered, Orange is > not liable for messages that have been modified, changed or > falsified. Thank you. > -- Andrew J Grimberg Manager Release Engineering The Linux Foundation -=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#20769): https://lists.onap.org/g/onap-discuss/message/20769 Mute This Topic: https://lists.onap.org/mt/73054581/21656 Group Owner: [email protected] Unsubscribe: https://lists.onap.org/g/onap-discuss/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
signature.asc
Description: OpenPGP digital signature
