| mnasiadka | noonedeadpunk: are you ok with gouthamr’s response in https://review.opendev.org/c/openstack/project-config/+/959904? Any team can override toggleWipState locally (in their acts) - so I think I’m leaning towards merging it | 05:21 |
|---|---|---|
| mnasiadka | In their acls, bloody correction mechanism :) | 05:22 |
| opendevreview | Merged openstack/openstack-manuals master: [www] Set 2024.2 Dalmatian as End of Life https://review.opendev.org/c/openstack/openstack-manuals/+/987150 | 06:06 |
| noonedeadpunk | mnasiadka: so override in order to allow or prohibit? | 07:05 |
| mnasiadka | noonedeadpunk: prohibit (or rather limit to a -core group or something like that) | 07:05 |
| noonedeadpunk | As I am not sure what consistency are we talking there - I can not unset as a core somebodey elses -W, can I | 07:05 |
| noonedeadpunk | So consistent behavior is actually to keep current default... | 07:06 |
| noonedeadpunk | but I guess whatever | 07:06 |
| noonedeadpunk | If only I see it this way, I am fine to agree to disagree | 07:06 |
| mnasiadka | Current default is a bit irritating to be frank, so either we go through all repos and grant -core group toggling WIP state (which is not the nicest job around) or we go with gouthamr’s proposal (and limit on per group if needed) | 07:07 |
| noonedeadpunk | Yeah, this is smth I get. And I already changed ACLs for OSA :D | 07:08 |
| noonedeadpunk | But if we're talking about consistency... | 07:08 |
| noonedeadpunk | Like the one thing I am kinda "afraid" of - I know bunch of people that are really humble, and not ready to show up result of their work | 07:09 |
| noonedeadpunk | So it is really hard to convince them to push changes to gerrit, because they are gonna be judged and reviewed | 07:09 |
| noonedeadpunk | And WIP is kinda a "safe harbour" for them | 07:10 |
| mnasiadka | Well, you can rebase or edit to get rid of workflow -1, wipstate is different - now in majority of repos only the owner can change it :) | 07:10 |
| noonedeadpunk | Right, rebase or edit indeed does work... | 07:10 |
| noonedeadpunk | Well, as I said, I am fine agree to disagree here I guess | 07:10 |
| mnasiadka | And around these humble people - if only gerritbot would not post URLs to patches that are WIP - I would agree :) | 07:10 |
| mnasiadka | So maybe let’s go with that and see if there will be any abusers of this - we can always go back | 07:11 |
| mnasiadka | And that’s in line with other comments on that patch | 07:13 |
| noonedeadpunk | sure | 07:40 |
| noonedeadpunk | riiight... I really wonder if we actually should patch gerritbot... | 07:41 |
| noonedeadpunk | as also WIP patches in IRC are not very meaningful, imo | 07:41 |
| noonedeadpunk | but that is completely different topic | 07:42 |
| frickler | well I often do look at wip patches when I notice them on IRC and even comment on them, so I wouldn't necessarily agree on this | 07:53 |
| gouthamr | if you’re thinking about the way gerrit is writing their APIs, the use case you have doesn’t make sense, noonedeadpunk .. you as core wanting to remove someone’s W-1.. | 08:14 |
| gouthamr | you can’t overwrite a label added by someone else; that would get complicated, and abused. | 08:14 |
| noonedeadpunk | oh, sure, I am not arguing that removing label makes sense | 08:15 |
| gouthamr | ty for discussing this btw and driving a resolution, mnasiadka | 08:15 |
| noonedeadpunk | but I am not sure that WIP should be treated differently from regular labels | 08:16 |
| noonedeadpunk | And then why not to disable WIP as a whole and keep using -W | 08:16 |
| gouthamr | I don’t know if that’s an option? | 08:16 |
| gouthamr | I don’t like it myself.. it’s easy to get yourself into a WIP without intending to | 08:17 |
| noonedeadpunk | Well. Given that gerrit is used not only for openstack... probably not | 08:17 |
| noonedeadpunk | I don't know tbh | 08:17 |
| noonedeadpunk | Maybe we can get an ACL or smth disabling/prohibiting it for all openstacvk projects | 08:18 |
| mnasiadka | In an ideal world WIP is nice, because you can mark your patch as being WIP - as long as you’re active | 08:21 |
| mnasiadka | But in cases when somebody picks up your patch after N months of inactivity on it, that process gets complicated | 08:21 |
| gouthamr | +1 | 08:23 |
| noonedeadpunk | the problem is that we can't drop -W and leave just positive option anyway... | 08:26 |
| noonedeadpunk | So we have 2 things serving around same purpose | 08:26 |
| mnasiadka | W-1 for me says do not merge this, and is used in other cases than pure WIP | 08:27 |
| noonedeadpunk | W-1 is most useful to abort gate jobs for me though. But I am not sure what -W is not covering that pure WIP does | 08:31 |
| kinrui | just a reminder, the workflow -1 label was something we added as a workaround when we moved our gerrit fork that had a wip flag, with the idea that we could get rid of workflow -1 eventually if gerrit added a wip feature upstream | 11:38 |
| kinrui | er, when we moved *off* our gerrit fork | 11:39 |
| sean-k-mooney | kinrui: whil tha tis true to do that properly we need to modify the acls on every repo so that core team meber can remove the wip flag | 16:57 |
| sean-k-mooney | also by defualt gerrit filters out wip patches in some dashboard | 16:58 |
| clarkb | I think part of the problem is the overuse of WIP | 16:58 |
| sean-k-mooney | i.e. if you just go to changes adn click on open it redirects to https://review.opendev.org/q/status:open+-is:wip | 16:58 |
| clarkb | often times it feels like it is used when the author isn't confident in the change and not when the author is trying to avoid feedback. But really the way gerrit handles it the idea is "avoid bothering people/no feedback and prevent merging" | 16:59 |
| clarkb | what the label addresses is prevent merging without the second bit | 16:59 |
| sean-k-mooney | right i think the "this is not ready to merge but i woudl like early feedback" is the way we use it most often | 17:00 |
| sean-k-mooney | typiclly i use [WIP] in the commit to also delinate this | 17:00 |
| sean-k-mooney | because often thirdpary ci system will not run if you have -W or wip set | 17:01 |
| sean-k-mooney | but i might want that feedback | 17:01 |
| sean-k-mooney | as a core reviewr i generally want ot see wip patches by default and i can do that by adjucting hte filter | 17:01 |
| sean-k-mooney | but what i cant do today without modifying the acls is take over a wip patch form a contibutor that started the work | 17:02 |
| sean-k-mooney | i.e. they pushed it with WIP then lsot interst and i could not remove it | 17:02 |
| sean-k-mooney | so today i fine the native feature harmful until we correct that | 17:03 |
| sean-k-mooney | noonedeadpunk: i would not like use to patch gerrit bot to avoid posting wip patches to irc | 17:03 |
| sean-k-mooney | i do want to see those so i cna give basic directional input on the patch if tis in an area that i care about | 17:04 |
| sean-k-mooney | im not really agaisnt changign the text of -W as shown in the ui to encuage it to be used only for "do not merge this" | 17:06 |
| sean-k-mooney | but we still need +W for ok to merge | 17:07 |
| sean-k-mooney | so the lable cant go away entirly | 17:07 |
| clarkb | at the same time it is trivial to repush a change. And typically when you do that for a wip change you don't lose much of value except for waiting on ci results (because typically there hasn't been a ton of review at that point) | 17:07 |
| clarkb | yes I like the intentional act of "please merge this" being independent of the code review state | 17:08 |
| sean-k-mooney | clarkb: the last tiem i tried that the wip flag was not cleared automaticly | 17:08 |
| clarkb | sean-k-mooney: you have to push a new change | 17:08 |
| sean-k-mooney | well atleast it was not cleard when i did a rebase of the patch via the ui | 17:08 |
| clarkb | then abandon the WIP change | 17:08 |
| sean-k-mooney | i didnt try doing it via git-review | 17:08 |
| sean-k-mooney | oh | 17:08 |
| clarkb | rebasing does not make a new change it merely updates the existing change with a new patchset that has a new parent | 17:08 |
| sean-k-mooney | wll the issue with that is you lose the orginal author | 17:08 |
| sean-k-mooney | maybe that is not true | 17:09 |
| clarkb | you shouldn't that will be preserved by git | 17:09 |
| clarkb | the gerrit change owner will change but the author shouldn't | 17:09 |
| sean-k-mooney | right so i woudl need to change the change id in the commit message which will change the commiter | 17:09 |
| sean-k-mooney | bu tyour right the author shoudl be perserved | 17:10 |
| clarkb | yes but not the author | 17:10 |
| sean-k-mooney | ya its a bit of a pain but workable | 17:10 |
| *** kinrui is now known as fungi | 22:44 | |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!