| *** carloss has quit IRC | 00:22 | |
| *** gyee has quit IRC | 00:51 | |
| *** ociuhandu has joined #openstack-infra | 01:16 | |
| *** ociuhandu has quit IRC | 01:28 | |
| *** hamalq has quit IRC | 01:53 | |
| *** ramishra has quit IRC | 02:09 | |
| *** ramishra has joined #openstack-infra | 02:26 | |
| *** ramishra has quit IRC | 02:31 | |
| *** ramishra has joined #openstack-infra | 02:32 | |
| *** zxiiro has quit IRC | 02:49 | |
| *** zul has quit IRC | 03:15 | |
| *** rcernin has quit IRC | 03:43 | |
| *** rcernin has joined #openstack-infra | 03:44 | |
| *** rlandy|rover|bbl has quit IRC | 03:47 | |
| *** ykarel has joined #openstack-infra | 04:15 | |
| *** vishalmanchanda has joined #openstack-infra | 04:29 | |
| *** ociuhandu has joined #openstack-infra | 04:51 | |
| *** ociuhandu has quit IRC | 04:56 | |
| *** whoami-rajat has joined #openstack-infra | 04:57 | |
| *** rcernin has quit IRC | 04:59 | |
| *** ralonsoh has joined #openstack-infra | 05:18 | |
| *** ysandeep|away is now known as ysandeep | 05:29 | |
| *** rcernin has joined #openstack-infra | 05:39 | |
| *** rcernin has quit IRC | 05:40 | |
| *** rcernin has joined #openstack-infra | 05:40 | |
| *** rcernin has quit IRC | 05:54 | |
| *** rcernin has joined #openstack-infra | 06:01 | |
| *** ociuhandu has joined #openstack-infra | 06:03 | |
| *** ociuhandu has quit IRC | 06:13 | |
| *** eolivare has joined #openstack-infra | 06:24 | |
| *** sboyron has joined #openstack-infra | 06:25 | |
| *** __ministry has quit IRC | 06:30 | |
| *** ociuhandu has joined #openstack-infra | 06:33 | |
| *** amoralej|off is now known as amoralej | 06:33 | |
| *** kopecmartin has quit IRC | 06:34 | |
| *** kopecmartin has joined #openstack-infra | 06:34 | |
| *** tinwood has quit IRC | 06:37 | |
| *** jcapitao has joined #openstack-infra | 06:38 | |
| *** tinwood has joined #openstack-infra | 06:40 | |
| *** lpetrut has joined #openstack-infra | 06:40 | |
| *** ociuhandu has quit IRC | 06:44 | |
| *** dklyle has quit IRC | 06:49 | |
| *** ociuhandu has joined #openstack-infra | 06:50 | |
| *** ociuhandu has quit IRC | 06:52 | |
| *** zbr has quit IRC | 06:56 | |
| *** zbr has joined #openstack-infra | 06:58 | |
| *** ykarel_ has joined #openstack-infra | 07:00 | |
| *** ykarel has quit IRC | 07:02 | |
| *** __ministry has joined #openstack-infra | 07:07 | |
| *** ykarel_ has quit IRC | 07:15 | |
| *** andrewbonney has joined #openstack-infra | 07:15 | |
| *** ykarel has joined #openstack-infra | 07:17 | |
| *** ociuhandu has joined #openstack-infra | 07:21 | |
| *** gfidente|afk is now known as gfidente | 07:27 | |
| *** jpena|off is now known as jpena | 07:30 | |
| *** ociuhandu has quit IRC | 07:30 | |
| *** rcernin has quit IRC | 07:31 | |
| *** rpittau|afk is now known as rpittau | 07:32 | |
| *** ociuhandu has joined #openstack-infra | 07:37 | |
| *** tosky has joined #openstack-infra | 07:39 | |
| *** rcernin has joined #openstack-infra | 07:48 | |
| *** rcernin has quit IRC | 07:53 | |
| *** lucasagomes has joined #openstack-infra | 08:07 | |
| *** derekh has joined #openstack-infra | 08:11 | |
| *** rcernin has joined #openstack-infra | 08:18 | |
| *** yamamoto has quit IRC | 08:34 | |
| *** mciecierski has joined #openstack-infra | 08:42 | |
| *** ykarel is now known as ykarel|lunch | 08:44 | |
| *** ykarel|lunch has quit IRC | 08:48 | |
| *** ociuhandu has quit IRC | 09:06 | |
| *** ociuhandu has joined #openstack-infra | 09:07 | |
| openstackgerrit | Thierry Carrez proposed openstack/ptgbot master: Fix ~add ~del ~clean commands https://review.opendev.org/c/openstack/ptgbot/+/786606 | 09:11 |
|---|---|---|
| *** yamamoto has joined #openstack-infra | 09:11 | |
| *** dave-mccowan has joined #openstack-infra | 09:14 | |
| *** rcernin has quit IRC | 09:20 | |
| *** yamamoto has quit IRC | 09:21 | |
| openstackgerrit | Thierry Carrez proposed openstack/ptgbot master: Fix airbag deployment on empty messages https://review.opendev.org/c/openstack/ptgbot/+/786608 | 09:25 |
| *** ykarel has joined #openstack-infra | 09:38 | |
| *** ociuhandu has quit IRC | 09:39 | |
| *** ociuhandu has joined #openstack-infra | 09:40 | |
| *** ociuhandu has quit IRC | 09:42 | |
| *** ociuhandu has joined #openstack-infra | 09:42 | |
| *** ociuhandu has quit IRC | 09:42 | |
| *** ociuhandu has joined #openstack-infra | 09:43 | |
| *** jcapitao has quit IRC | 09:45 | |
| *** jcapitao has joined #openstack-infra | 09:46 | |
| *** ociuhandu has quit IRC | 09:53 | |
| *** ociuhandu has joined #openstack-infra | 10:02 | |
| *** dtantsur|afk is now known as dtantsur | 10:04 | |
| *** yamamoto has joined #openstack-infra | 10:05 | |
| *** yamamoto has quit IRC | 10:22 | |
| *** jcapitao is now known as jcapitao_lunch | 10:26 | |
| *** carloss has joined #openstack-infra | 10:31 | |
| *** ysandeep is now known as ysandeep|afk | 10:53 | |
| *** dave-mccowan has quit IRC | 11:08 | |
| *** cloudnull has quit IRC | 11:09 | |
| *** cloudnull7 has joined #openstack-infra | 11:09 | |
| *** dave-mccowan has joined #openstack-infra | 11:11 | |
| *** ociuhandu has quit IRC | 11:20 | |
| *** rcernin has joined #openstack-infra | 11:24 | |
| *** yamamoto has joined #openstack-infra | 11:28 | |
| *** jpena is now known as jpena|lunch | 11:32 | |
| *** ysandeep|afk is now known as ysandeep | 11:44 | |
| *** psachin has quit IRC | 11:48 | |
| *** rlandy has joined #openstack-infra | 11:50 | |
| *** rlandy is now known as rlandy|rover | 11:50 | |
| *** ociuhandu has joined #openstack-infra | 11:50 | |
| *** ociuhandu has quit IRC | 11:56 | |
| *** ociuhandu has joined #openstack-infra | 12:00 | |
| *** ociuhandu has quit IRC | 12:04 | |
| *** jcapitao_lunch is now known as jcapitao | 12:09 | |
| *** ociuhandu has joined #openstack-infra | 12:20 | |
| *** jpena|lunch is now known as jpena | 12:30 | |
| *** ociuhandu has quit IRC | 12:36 | |
| *** ociuhandu has joined #openstack-infra | 12:37 | |
| *** auristor has quit IRC | 12:38 | |
| *** auristor has joined #openstack-infra | 12:42 | |
| *** ociuhandu has quit IRC | 12:42 | |
| *** rcernin has quit IRC | 12:47 | |
| *** ociuhandu has joined #openstack-infra | 12:50 | |
| *** nweinber has joined #openstack-infra | 13:00 | |
| *** ociuhandu has quit IRC | 13:10 | |
| *** amoralej is now known as amoralej|lunch | 13:18 | |
| openstackgerrit | Jeremy Stanley proposed openstack/ptgbot master: Use Opendev Etherpad site https://review.opendev.org/c/openstack/ptgbot/+/780668 | 13:25 |
| openstackgerrit | Merged openstack/ptgbot master: Fix ~add ~del ~clean commands https://review.opendev.org/c/openstack/ptgbot/+/786606 | 13:30 |
| openstackgerrit | Merged openstack/ptgbot master: Fix airbag deployment on empty messages https://review.opendev.org/c/openstack/ptgbot/+/786608 | 13:33 |
| *** ociuhandu has joined #openstack-infra | 13:40 | |
| *** dchen has quit IRC | 13:41 | |
| *** yamamoto has quit IRC | 13:43 | |
| *** ociuhandu has quit IRC | 13:46 | |
| *** ysandeep is now known as ysandeep|away | 13:47 | |
| *** bhagyashris is now known as bhagyashris|away | 13:49 | |
| *** amoralej|lunch is now known as amoralej | 13:51 | |
| *** hashar has joined #openstack-infra | 14:01 | |
| *** ykarel has quit IRC | 14:03 | |
| *** dasm has quit IRC | 14:13 | |
| *** rpittau is now known as rpittau|afk | 14:18 | |
| *** dasm has joined #openstack-infra | 14:25 | |
| *** lpetrut has quit IRC | 14:26 | |
| *** yamamoto has joined #openstack-infra | 14:27 | |
| *** ociuhandu has joined #openstack-infra | 14:32 | |
| *** dklyle has joined #openstack-infra | 14:32 | |
| *** jamesdenton has quit IRC | 14:36 | |
| *** yamamoto has quit IRC | 14:37 | |
| *** mciecierski has quit IRC | 14:46 | |
| *** dviroel is now known as dviroel|lunch | 14:48 | |
| *** ociuhandu has quit IRC | 14:59 | |
| *** ociuhandu has joined #openstack-infra | 14:59 | |
| *** jamesdenton has joined #openstack-infra | 15:13 | |
| *** ykarel has joined #openstack-infra | 15:35 | |
| *** rlandy|rover is now known as rlandy|rover|tra | 15:40 | |
| *** rlandy|rover|tra is now known as rlandy|rvr|train | 15:40 | |
| *** hashar has quit IRC | 15:41 | |
| *** amoralej is now known as amoralej|off | 15:43 | |
| *** dviroel|lunch is now known as dviroel | 15:55 | |
| *** lucasagomes has quit IRC | 16:00 | |
| *** ykarel has quit IRC | 16:01 | |
| *** ykarel has joined #openstack-infra | 16:02 | |
| openstackgerrit | Jeremy Stanley proposed openstack/project-config master: Allow inheritFrom in Gerrit configs https://review.opendev.org/c/openstack/project-config/+/786685 | 16:08 |
| openstackgerrit | Jeremy Stanley proposed openstack/project-config master: OpenStack Release Team ACLs inherit meta-config https://review.opendev.org/c/openstack/project-config/+/786686 | 16:08 |
| *** ociuhandu_ has joined #openstack-infra | 16:22 | |
| *** rlandy|rvr|train is now known as rlandy|rover | 16:23 | |
| *** dtantsur is now known as dtantsur|afk | 16:24 | |
| *** ociuhandu has quit IRC | 16:26 | |
| *** ociuhandu_ has quit IRC | 16:27 | |
| *** hamalq has joined #openstack-infra | 16:28 | |
| *** hamalq has quit IRC | 16:28 | |
| *** hamalq has joined #openstack-infra | 16:29 | |
| *** ociuhandu has joined #openstack-infra | 16:31 | |
| *** jpena is now known as jpena|off | 16:31 | |
| *** ociuhandu has quit IRC | 16:36 | |
| *** ykarel has quit IRC | 16:42 | |
| *** ralonsoh has quit IRC | 16:53 | |
| openstackgerrit | Merged openstack/ptgbot master: Use Opendev Etherpad site https://review.opendev.org/c/openstack/ptgbot/+/780668 | 16:54 |
| *** jcapitao has quit IRC | 16:55 | |
| *** gyee has joined #openstack-infra | 17:04 | |
| *** derekh has quit IRC | 17:08 | |
| *** hemna has quit IRC | 17:10 | |
| *** hemna has joined #openstack-infra | 17:11 | |
| *** hemna has quit IRC | 17:15 | |
| *** hemna has joined #openstack-infra | 17:26 | |
| *** mciecierski has joined #openstack-infra | 17:29 | |
| *** mciecierski has quit IRC | 17:31 | |
| *** ajitha has joined #openstack-infra | 17:40 | |
| *** vishalmanchanda has quit IRC | 18:04 | |
| *** andrewbonney has quit IRC | 18:05 | |
| *** gfidente is now known as gfidente|afk | 18:21 | |
| openstackgerrit | Jeremy Stanley proposed openstack/project-config master: OpenStack release-test ACLs inherit meta-config https://review.opendev.org/c/openstack/project-config/+/786686 | 18:37 |
| *** eolivare has quit IRC | 18:38 | |
| *** whoami-rajat has quit IRC | 18:51 | |
| *** rlandy|rover is now known as rlandy|rvr|train | 18:56 | |
| openstackgerrit | Merged openstack/pbr master: Map requires-python to python-requires (attempt 2) https://review.opendev.org/c/openstack/pbr/+/780633 | 19:00 |
| openstackgerrit | Merged openstack/project-config master: Allow inheritFrom in Gerrit configs https://review.opendev.org/c/openstack/project-config/+/786685 | 19:11 |
| openstackgerrit | Merged openstack/project-config master: OpenStack release-test ACLs inherit meta-config https://review.opendev.org/c/openstack/project-config/+/786686 | 19:11 |
| *** dciabrin_ has joined #openstack-infra | 19:40 | |
| *** dciabrin has quit IRC | 19:42 | |
| *** ajitha has quit IRC | 19:43 | |
| *** smcginnis has quit IRC | 19:54 | |
| *** rlandy|rvr|train is now known as rlandy|rover | 20:04 | |
| *** slaweq has quit IRC | 20:29 | |
| *** nweinber has quit IRC | 20:36 | |
| *** zzzeek has quit IRC | 20:38 | |
| *** zzzeek has joined #openstack-infra | 20:44 | |
| *** yamamoto has joined #openstack-infra | 20:50 | |
| *** sboyron has quit IRC | 21:08 | |
| openstackgerrit | Elod Illes proposed openstack/project-config master: Move projects under meta-config acl https://review.opendev.org/c/openstack/project-config/+/786735 | 21:22 |
| *** yamamoto has quit IRC | 21:29 | |
| *** kkalina has joined #openstack-infra | 21:34 | |
| *** kkalina has quit IRC | 21:35 | |
| *** kkalina has joined #openstack-infra | 21:36 | |
| *** mfuller has joined #openstack-infra | 21:42 | |
| *** mfuller has quit IRC | 21:43 | |
| *** mfuller has joined #openstack-infra | 21:44 | |
| *** rlandy|rover has quit IRC | 21:49 | |
| kkalina | Hi, we need to change rewrite master branch in our project airship/treasuremap . To do so we need force push right or atleast ability to delete a branch and pushh a new one (delete/create) master. I think force push is prefered. | 21:55 |
| kkalina | To get this rights, do we need to create a special group for the repository, lets say airship-owners? and then grant its ACL ability to force push? | 21:55 |
| clarkb | kkalina: why do you need to force rewrite master? was development not performed there? | 21:55 |
| clarkb | eg this is a catch up to some forked state>? | 21:56 |
| kkalina | Thank you for swift reply. One second let me explain the reason | 21:56 |
| clarkb | I want to make sure we understand the problem before we decide on a solution | 21:56 |
| fungi | ideally you'd change the contents of the branch through reverting whatever you didn't mean to merge to it | 21:58 |
| kkalina | airship/treasuremap had a master branch for couple of years, and all development was there (with other tags and branches) | 21:58 |
| kkalina | At some point it was decided that we need a complete redesign, from top to bottom, so we created a branch called v2. It has completely different code base, and has nothing in common with v1, so it doesn't have a point in git history where they were the same | 21:58 |
| fungi | force push will result in non-fast-forwardable states which will confuse consumers of that branch (users, ci systems...) | 21:58 |
| kkalina | we want to deprecate whatever is in v1 | 21:59 |
| kkalina | and rewrite history | 21:59 |
| clarkb | kkalina: you may be better off pushing a merge commit instead | 21:59 |
| kkalina | they dont have anythhing in common in terms of git history or code base | 21:59 |
| clarkb | you can use a merge strategy that doesn't keep any content from the other side, but it keeps the history happy for users, ci etc | 21:59 |
| clarkb | kkalina: yes I udnerstand that, but git specifically has tooling for these scenarios | 21:59 |
| kkalina | we dont want users to have v1 commit history. | 22:00 |
| clarkb | on master `git merge -s theirs v2` I think | 22:00 |
| clarkb | kkalina: well you shouldn't just delete that history? | 22:00 |
| clarkb | then you can push that merge commit to master and land it, no extra permissiosn needed if you already have perms to push merge commits | 22:01 |
| clarkb | and you won't need to force reset the repo state downstream | 22:01 |
| fungi | git merge --strategy=ours origin/v2.0 master | 22:01 |
| fungi | or you can merge the other direction | 22:01 |
| openstackgerrit | Elod Illes proposed openstack/project-config master: Move projects under meta-config acl (2) https://review.opendev.org/c/openstack/project-config/+/786739 | 22:02 |
| kkalina | give me one second, so i understand this right | 22:02 |
| clarkb | (my command is wrong, theirs is an option not a strategy name) | 22:02 |
| kkalina | i dont think that git merge does what we want to achieve, but let me double check with the strategy | 22:02 |
| clarkb | kkalina: there is definitely a way to express this in git, though my specific command may be wrong | 22:03 |
| clarkb | essentially you end up saying we had two states on different branches before but now there is one state that reflects only one of the two branches before | 22:03 |
| clarkb | there are two major upsides to representing things this way in git. First you don't lose any of your old history. Second users can git pull without doing a hard reset | 22:04 |
| openstackgerrit | Elod Illes proposed openstack/project-config master: Move projects under meta-config acl (3) https://review.opendev.org/c/openstack/project-config/+/786740 | 22:04 |
| clarkb | fungi's command is correct I think "This resolves any number of heads, but the resulting tree of the merge is always that of the current branch head, effectively ignoring all changes from all other branches. It is meant to be used to supersede old development history of side branches. Note that this is different from the -Xours option to the recursive merge strategy." | 22:04 |
| kkalina | let me stress, that we dont want git history from v1 in master. because it has no relationship with the code base | 22:04 |
| clarkb | kkalina: I understand that, but this is the entire point of git history | 22:05 |
| kkalina | the old master will leave in new branch | 22:05 |
| clarkb | kkalina: you can rewrite it, but the history is still there. | 22:05 |
| fungi | there's no "theirs" strategy btw, it's --strategy=recursive --strategy-option=theirs | 22:05 |
| clarkb | fungi: yup, your command is correct, mine was wrong | 22:05 |
| clarkb | kkalina: you can also use a new branch for the v1 tree. You can do both things is essentially what I'm suggesting and they will make people's lives easier | 22:06 |
| fungi | kkalina: if you have none of the original master history in the resulting branch state, it will be non-fast-forwardable | 22:06 |
| fungi | people (and automated systems) won't be able to pull from master across that event horizon | 22:07 |
| kkalina | i am trying to understand usecase for someone wanting to use master and use some v1 from it, instead of a designed branch for it | 22:09 |
| fungi | kkalina: the use case is `git pull` | 22:09 |
| clarkb | kkalina: the main reason is as fungi points out, to ensure there isn't a non fastforwardable state | 22:09 |
| fungi | you will break git pull | 22:09 |
| clarkb | it to ensure that git itself continues to oeprate on master successfully | 22:10 |
| clarkb | note treasuremap doesn't seem to have the push merge permission on it. My suggestion is that that permission get added then a git merge -s ours version of v2 into master gets pushed to master as a merge commit and that is landed | 22:10 |
| kkalina | so you mean, that if someone had "old master" cloned, he wont be able to do git pull anymore? | 22:10 |
| clarkb | correct | 22:11 |
| fungi | if they had ever cloned the repository they won't be able to git pull in master | 22:11 |
| fungi | master is your default branch, so they will have a master branch state it they ever cloned the repo | 22:11 |
| fungi | if you "rewrite history" with push --force, it will also potentially break git mirroring, require cleanup in ci systems to force new branch states, and need confusing instructions for anyone who has an existing copy of the repository to reset it | 22:12 |
| kkalina | these are valid points worth considering | 22:13 |
| kkalina | let me think about this | 22:13 |
| kkalina | but in any case. if we want to allow merges | 22:14 |
| kkalina | can u give an example where its added to other projects? | 22:14 |
| clarkb | kkalina: https://opendev.org/openstack/project-config/src/branch/master/gerrit/acls/openstack/swift.config#L1-L2 you need an acl that looks like that in https://opendev.org/openstack/project-config/src/branch/master/gerrit/acls/airship/treasuremap.config | 22:14 |
| fungi | yes, allowing your release team group to push merge commits for review can be useful for merging between branches | 22:14 |
| fungi | note we don't recommend allowing all users to push merge commits, because it's too easy to accidentally push a dirty branch state for review | 22:15 |
| clarkb | as long as teh v2 development happened in gerrit so gerrit knows about those commits already then you should be able to push a merge change liek we describe to master. | 22:15 |
| clarkb | this merge commit will be in review, so you can check it before you merge it too | 22:16 |
| clarkb | note that zuul went through something similar when v2 -> v3 happened. Except in that case we didn't do a wholesale replacement it was merged back in more typically | 22:17 |
| clarkb | but still the v2 history is there and relevant and we avoided breaking existing clones | 22:17 |
| kkalina | so we should create a new group, lets say "airship-release" and grant it the permissions for the merge? | 22:18 |
| clarkb | there is already an airship-release group https://opendev.org/openstack/project-config/src/branch/master/gerrit/acls/airship/treasuremap.config#L3 I think you just need to grant them the permissions | 22:19 |
| fungi | right. often it will be the same group you allow to push signed tags | 22:19 |
| kkalina | ok, let me try to do this | 22:19 |
| kkalina | thank you for valuable advice | 22:19 |
| fungi | yeah, the end result is that git has the history of what you actually did: replaced the content in master with the what was in the v2.0 branch including all its history, but that prior to that merge point the content (and history) of the master branch was what it was before | 22:20 |
| clarkb | also note that you don't want to try using theirs, it isn't forceful enough you what --strategy ours. `git checkout v2 && git merge -s ours master && edit .gitreview file to push to master && git review` | 22:21 |
| clarkb | something like that I think. fungi ^ does that look right to you? | 22:21 |
| fungi | the example in the git-merge manpage has: `git merge -s ours obsolete` "Merge branch obsolete into the current branch, using ours merge strategy" | 22:23 |
| clarkb | ya I think I got it right then as master is obsolete in this case | 22:23 |
| clarkb | then just have to make sure you push tothe right target which is master in this case | 22:23 |
| clarkb | well refs/for/master :) | 22:24 |
| fungi | the other gotcha is that you'll also need to tack on a commit to update the defaultbranch in .gitreview | 22:24 |
| clarkb | you don't need to commit it I don't think, just edit it | 22:25 |
| clarkb | but you'll want to do that anyway I suppose | 22:25 |
| clarkb | since future dev wants to go to master | 22:25 |
| fungi | you'll eventually want a change in review to update the .gitreview file yes | 22:25 |
| fungi | whether you commit and push it right away or later | 22:25 |
| clarkb | ++ | 22:26 |
| kkalina | looking at the examples: | 22:26 |
| kkalina | ``` | 22:26 |
| kkalina | [access "refs/for/refs/*"] | 22:26 |
| kkalina | pushMerge = group swift-milestone | 22:26 |
| kkalina | ``` | 22:26 |
| kkalina | i am a bit confused with refs/for/refs/ | 22:26 |
| fungi | kkalina: refs/for/refs/ is where changes for review are pushed | 22:26 |
| fungi | so that's allowing merge commits to be pushed as changes for review | 22:26 |
| clarkb | its the special ref that tells gerrit this push is being proposed for review | 22:26 |
| kkalina | so refs/for/master | 22:26 |
| fungi | they'll show up like a normal change you can review and decide whether to approve | 22:26 |
| clarkb | refs/for/refs/master in this case I think | 22:26 |
| clarkb | when you push to ^ gerrit knows this is a proposed change to refs/master. If the change is merged it gets applied to refs/master | 22:27 |
| fungi | i wouldn't suggest getting more specific than refs/for/refs/* in the acl though | 22:27 |
| kkalina | ok let me try | 22:27 |
| fungi | note we document this at https://docs.opendev.org/opendev/infra-manual/latest/drivers.html#merge-commits | 22:28 |
| fungi | though we don't document complete replacement using --strategy=ours as that's a fairly uncommon need | 22:30 |
| *** yamamoto has joined #openstack-infra | 22:30 | |
| openstackgerrit | Kostyantyn Kalynovskyi proposed openstack/project-config master: ACL, allow merge for airship-release group https://review.opendev.org/c/openstack/project-config/+/786741 | 22:32 |
| kkalina | thank you for your help, and knowledge sharing :) | 22:34 |
| fungi | you're welcome, we're around if you need more assistance on this | 22:34 |
| kkalina | we'll add merge request soon | 22:34 |
| fungi | note once that acl change merges (i've approved it) there's a bit of a delay before our continuous deployment applies it, but you should be able to tell by looking at https://review.opendev.org/admin/repos/airship/treasuremap,access while logged in as a member of your airship-release group | 22:36 |
| *** gyee has quit IRC | 22:37 | |
| fungi | and if you edit the .gitreview file after creating the merge commit, as clarkb suggests, then `git review` should push it for the correct branch automatically for you | 22:38 |
| fungi | this should also actually trigger your usual ci jobs, respecting any local changes in your zuul.d/*.yaml files from the v2 branch | 22:39 |
| kkalina | i dont use git review, but this is useful anyway :) | 22:39 |
| fungi | yeah, if you're not using git-review, then push to refs/for/refs/master as discussed earlier | 22:39 |
| kkalina | (y) | 22:40 |
| clarkb | out of curiousity what tool do you use? | 22:43 |
| clarkb | or do you just git push gerrit refs/for/refs/master ? | 22:43 |
| clarkb | er would be `git push gerrit HEAD:refs/for/refs/master` | 22:44 |
| clarkb | one of the reasons the acls are setup the way they are is to try and force people twoards these solutions too :) | 22:45 |
| clarkb | we don't want them to get in your way, but also want to avoid making mistakes that will be painful afterwards | 22:45 |
| kkalina | during normal developement i use `git push origin HEAD:refs/for/master | 22:46 |
| kkalina | but that is just my preference | 22:46 |
| kkalina | because i did not work with gerrit too much before | 22:46 |
| clarkb | kkalina: thats fine. Probably the biggest reason to not do that is git-review will try and catch merge conflicts against the target branch early which can be a nice feature but not necesasry | 22:47 |
| fungi | yeah, refs/for/master and refs/for/refs/master and for/refs/master and for/master probably all work synonymously (assuming you don't have anything shadowing them) | 22:47 |
| clarkb | git review also warns you when you are about to push up many changes allowing you to review them and be sure that is what you want | 22:47 |
| clarkb | but these are just additional helpers and not strictly required | 22:48 |
| fungi | one other potential gotcha, i think you may need to git commit --amend after the merge so that the commit hook kicks in to add the change-id footer in the commit message of the merge commit | 22:48 |
| kkalina | lots of ammends | 22:48 |
| kkalina | :) | 22:48 |
| *** yamamoto has quit IRC | 22:49 | |
| *** yamamoto has joined #openstack-infra | 22:49 | |
| kkalina | i noticed that git review, rewrites the whole commit chain somtimes, when i need to change only the last commit in the chain | 22:49 |
| openstackgerrit | Merged openstack/project-config master: ACL, allow merge for airship-release group https://review.opendev.org/c/openstack/project-config/+/786741 | 22:49 |
| fungi | yeah, the change-id generation is automated in a commit hook, but git tends to only call that on `git commit` and not on other git commands which create commits like revert or merge | 22:50 |
| fungi | kkalina: git-review should never rewrite changes | 22:50 |
| fungi | it will prompt you when you have multiple commits in a series that you're about to push them all, but gerrit will only update the ones which are actually new/altered commits | 22:50 |
| kkalina | probably i did something wrong with review. | 22:51 |
| fungi | so if you have a series of 3 in your branch and only alter the most recent commit, git-review will warn you you're about to push 3 commits, but the first two are unchanged so gerrit doesn't update them | 22:51 |
| clarkb | old git review did that actually | 22:52 |
| clarkb | but very old | 22:52 |
| fungi | if you performed a rebase of your commits, then that will rewrite them, but that's git doing it not git-review | 22:52 |
| fungi | and there are options you can pass to git-review to ask it to rebase, but they're off by default and not recommended | 22:52 |
| clarkb | now the only thing it should do is attempt a rebase, if it rebases clean it undoes it. If it fails then git review fails and says you need to manualyl rebase to address the problems | 22:52 |
| clarkb | this is how it detects if you have merge conflicst against the target | 22:53 |
| fungi | yeah it performs a local test rebase to check for possible merge conflicts, but then unwinds it back to the earlier state before pushing | 22:53 |
| clarkb | another related thing that trips people up is they often don't realize that a cherry-pick always creatse a new commit | 22:53 |
| clarkb | (because it updatse the timestamp) | 22:54 |
| *** yamamoto has quit IRC | 22:54 | |
| kkalina | thats nice to know, about the timestamp | 22:54 |
| clarkb | ya everything else stays the same but then it bumps the timestamp and that causes confusion occasionally | 22:55 |
| fungi | well, because the commit time is part of what's hashed to create the commit id | 22:56 |
| clarkb | yup | 22:56 |
| kkalina | so we managed to create this patchset https://review.opendev.org/c/airship/treasuremap/+/786742 | 23:06 |
| kkalina | trying to understand if it does what we actually want | 23:06 |
| clarkb | kkalina: there are a number of thinsg to look at. First is the target branch which gerrit shows is master (so that is good) | 23:08 |
| fungi | one way to check is to fetch the change from gerrit and see if it corresponds to the state you want | 23:08 |
| clarkb | next is the two parent commits : https://review.opendev.org/plugins/gitiles/airship/treasuremap/+/b7e91e71ca2968b2573a5ac6ca9bb02a9488c2a9 and https://review.opendev.org/plugins/gitiles/airship/treasuremap/+/0f0c7cc4e6f50ad71c26fe37a49a786a83865b5d | 23:08 |
| clarkb | but ya ultimately I would fetch the change back from gerrit and see if it reflects the state you expect to see | 23:09 |
| kkalina | great idea, let me check | 23:09 |
| fungi | the gerrit webui has improved in this regard, but it's still not great for actually evaluating whether a merge commit is what you intend | 23:09 |
| clarkb | ya in particular you want to ensure you got the ours relationship correct | 23:09 |
| clarkb | and gerrit doesn't really seem to give that to us | 23:10 |
| clarkb | I think the ours may have beend done backwards, but its hard for me to be sure as I'm not super familiar with the v1 and v2 branches | 23:12 |
| kkalina | yes, i have that feeling as well | 23:12 |
| clarkb | ya .github isn't on v2 or v2.0 but it is in master and in the result of that merge | 23:13 |
| clarkb | which makes me think this may have been done backwards | 23:14 |
| clarkb | (I think the way git and gerrit try to represent that is in the ordering of the merge parents. The master commit is ordered first before the v2 parent) | 23:14 |
| kkalina | seems like the files are actually from old master | 23:17 |
| kkalina | instead of v2 | 23:17 |
| clarkb | yup | 23:17 |
| clarkb | that implies to me that the merge was done the wrong way around | 23:17 |
| fungi | yeah, if i checkout origin/v2.0 and then `git merge -s ours origin/master` i get a branch state with no .github directory | 23:17 |
| clarkb | you should be on v2 when doing git merge -s ours master | 23:17 |
| clarkb | but this would've been done as checkout master and git merge -s ours v2 I think | 23:17 |
| fungi | kkalina: 786743 looks like what you want | 23:29 |
| *** yamamoto has joined #openstack-infra | 23:29 | |
| kkalina | yet, looks right to me | 23:30 |
| kkalina | thank you for looking into this, you really helped a lot | 23:32 |
| fungi | no problem, always happy to help | 23:33 |
| fungi | hopefully the ci jobs produce expected results on that review | 23:33 |
| fungi | and then you should be able to approve it like any other change once you're satisfied | 23:33 |
| fungi | and the end result should be that, once it lands, anyone doing a git pull in master will update to the same state as what was in the other branch as of the time of the merge | 23:35 |
| fungi | i should be around for at least a few more hours if you have any more questions | 23:36 |
| *** tosky has quit IRC | 23:38 | |
| *** ociuhandu has joined #openstack-infra | 23:49 | |
| *** ociuhandu has quit IRC | 23:54 | |
| *** ysirndjuro has quit IRC | 23:55 | |
| *** ysandeep|away is now known as ysandeep | 23:55 | |
| *** yamamoto has quit IRC | 23:56 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!