*** ysandeep|out is now known as ysandeep | 05:58 | |
*** ysandeep is now known as ysandeep|brb | 06:42 | |
*** amoralej|off is now known as amoralej | 07:02 | |
*** ysandeep|brb is now known as ysandeep | 07:22 | |
*** odyssey4me is now known as Guest1217 | 08:32 | |
*** jpena|off is now known as jpena | 08:38 | |
*** dmellado_ is now known as dmellado | 09:45 | |
*** ysandeep is now known as ysandeep|coffee | 10:04 | |
*** ysandeep|coffee is now known as ysandeep | 10:51 | |
*** rlandy is now known as rlandy|ruck | 11:14 | |
*** amoralej is now known as amoralej|lunch | 13:10 | |
*** dasm|off is now known as dasm | 13:19 | |
*** dasm is now known as dasm|rover | 13:19 | |
*** amoralej|lunch is now known as amoralej | 13:54 | |
*** dviroel is now known as dviroel|lunch | 15:39 | |
*** jpodivin_ is now known as jpodivin | 15:43 | |
sean-k-mooney | clarkb: fungi ye did a gerrit update recently yes. i am seing a change in behavior that im somewhat concerned about form a ux persepctive | 15:46 |
---|---|---|
sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/825496/9/nova/tests/unit/virt/libvirt/test_driver.py | 15:46 |
*** ysandeep is now known as ysandeep|dinner | 15:46 | |
sean-k-mooney | im not sure if this always happesn or if gerrit is now smart enouch to forward port thigns but i left that comment on patchest 7 | 15:47 |
sean-k-mooney | not on 9 | 15:47 |
sean-k-mooney | and i was expecting it to be show on 7 | 15:47 |
sean-k-mooney | now in this case it still applies to the latest revision | 15:47 |
sean-k-mooney | so if gerrit is only forward porting it for cases where that is the case this might not be a major regression | 15:48 |
sean-k-mooney | but if it always puts comment only on the latest version or has decoupled them from the reviosn the way github does that is a major regressions in fucntiaonaltiy | 15:48 |
sean-k-mooney | once which i think we should look at disabling if its configurable | 15:48 |
sean-k-mooney | it does seam to also leave the commment on the old revsion so it think it should be ok | 15:49 |
sean-k-mooney | its just a little concerning to see this change. was that somethign ye were aware of before the update | 15:50 |
fungi | sean-k-mooney: there's an ongoing discussion about this upstream | 15:51 |
clarkb | and that behavior existed in3.3 as well | 15:51 |
clarkb | I don't know if it was in 3.2 | 15:51 |
fungi | sean-k-mooney: https://groups.google.com/g/repo-discuss/c/evFgN3tTwxE | 15:51 |
clarkb | oh thanks I was going to dig up links | 15:52 |
sean-k-mooney | i think as long as it still reports on the old one and the comment still applies to the new version its proably workable | 15:52 |
sean-k-mooney | if it was on the latest patch only that would be a problem | 15:52 |
fungi | sean-k-mooney: the short story is that gerrit expects you to explicitly "resolve" those comments | 15:52 |
fungi | then it won't show them | 15:52 |
fungi | it only shows unresolved comments from earlier patchsets | 15:52 |
sean-k-mooney | right we dont expect you do do that really | 15:53 |
sean-k-mooney | we dont expect you to ever have to use the resolved checkbox in our workflow today | 15:53 |
fungi | agreed, that's the crux of the discussion. potential compromises between those workflows | 15:53 |
fungi | we can probably find a happy medium | 15:53 |
fungi | i suggested that if the lines being commented on change in a later patchset, that could be considered implicit resolution | 15:54 |
sean-k-mooney | am perhaps but its not really about makeing them resovled as such that i am worried about | 15:55 |
sean-k-mooney | we sometime have a lot of discusin on a partical revision | 15:55 |
sean-k-mooney | wehre we have several converation in differnt parts of the file or change | 15:55 |
sean-k-mooney | for context we often continue them on the older revsion | 15:56 |
sean-k-mooney | while the owner adresses some of them in later revisions | 15:56 |
sean-k-mooney | eventurally the conversation moves to the latest revision | 15:56 |
clarkb | yes I think that workflow is why they want you explicitly resolve things | 15:56 |
clarkb | because the discussion may span multiple patchsets and they want to show that relevant info until you mark it done | 15:56 |
sean-k-mooney | right but we dont neesaly want the discussion to move to the latest revsion | 15:57 |
clarkb | But making that work currently requires that you explicitly manage your comment states when they change state | 15:57 |
sean-k-mooney | unless all comment are forward ported but that seam noisy | 15:57 |
sean-k-mooney | as the comment would have to be forward ported when the patch was uploaded | 15:57 |
clarkb | sean-k-mooney: ya the ones you don't wnat to move should be marked resolved. Those that aren't resolved will move forward until resolved is the idea behind this | 15:57 |
clarkb | The problem previously raised is that people don't know to do that and even when they do know to do it they find it to be additional effort. So current discussion has trended towards finding hueristics to reduce the need for manual intervention | 15:58 |
sean-k-mooney | ideally we woudl just have a per project option to maintain the old behavior | 15:59 |
clarkb | or even per user | 16:00 |
clarkb | that would be worth suggesting on the issue if you can articulate why it works better for nova's typical workflow | 16:00 |
sean-k-mooney | as an authour i guess if i am respoond ot a lot of feedback i do use the ack or done button to keep track of what i have done somethinme as im adressing comment in my code or spec | 16:00 |
sean-k-mooney | but for anyone else it seams weired to use the resovled checkbox | 16:00 |
clarkb | all the resolved box means is this train of thought is done we don't need to keep tracking it anymore | 16:01 |
clarkb | and done comments auto resolve | 16:01 |
sean-k-mooney | clarkb: rgiht so to me that does not really add much value | 16:01 |
sean-k-mooney | i dont thinke comment resoltion trackign is really that useful because if i am rerviewing the change im not going to just take the authors word that the comment has been adressed im going to go commment by comment and ensure they resolved it correctly | 16:02 |
clarkb | the value is in the explicitness of what is outstanding. I can totally see how it add a ton of value if everyone is doing it | 16:02 |
clarkb | The problem is we've got a lot of old timer gerrit users who never had to do that and we've got a lot of new gerrit users who are happy just to push changes. And neither of those groups are in a good position to do that work | 16:03 |
clarkb | (I mentioned this in the thread too) | 16:03 |
sean-k-mooney | i have seen people mark things as done before when they have not | 16:03 |
clarkb | its not about taking the authors word. Its about managing the priority information in the UI | 16:03 |
clarkb | but you are right anyone can mark anything done and then potentially disrupt the flow in the opposite direction (lost info instead of too much info) | 16:04 |
sean-k-mooney | clarkb: well my poin is that info might be misleading | 16:04 |
fungi | yeah, i think the workflow the gerrit authors envision is that reviewer comments accumulate as a sort of checklist, and they want to see the change author address the individual items on that checklist before it can be approved | 16:04 |
clarkb | it requires everyone to act in good faith and I think we have that overall with our users. Its more that its a new habit for old users and unexpected for new users | 16:04 |
sean-k-mooney | clarkb: its generaly new contibutors that i see not adressing comment propelry to be fair | 16:05 |
fungi | also gerrit has been moving toward a "threaded" discussion model, and they want threads to carry through from one patchset to the next | 16:05 |
clarkb | oh I mean I'm an old gerrit user that almost never marks things done because I just don't have that habit. I don't need to accuse anyone else of that I do it myself :) | 16:05 |
clarkb | I'm sure I can force myself to change habits but then when you think of changing 100 other people's habits I can definitely see how auto resolution in some cases would be useful. Or making it configurable by user | 16:06 |
sean-k-mooney | clarkb: well i also rarely do it for my onw changes unless i have more then 20 comments to adress | 16:06 |
clarkb | basically I can see why they haev made the changes they do and I can see why they are beneficial. We just need to find ways to get some of those benefits if not a super tight nit work in same office group of reviewers following a strict pattern like the android devs | 16:06 |
*** ysandeep|dinner is now known as ysandeep | 16:06 | |
clarkb | the good news is we're now close enough to gerrit master we can provide this feedback and it is actionable upstream | 16:07 |
clarkb | unlike before when we were many years behind and any info was a dead end bceause we needed to upgrade | 16:07 |
sean-k-mooney | ya i guess. in general however lots of the change they have made recently like the work in progress state vs or work in progress lables have seamed detrimatl to our usage | 16:07 |
clarkb | I disagree | 16:08 |
sean-k-mooney | well now we have 2 ways to mark things as work in progress, well 3 | 16:08 |
clarkb | sean-k-mooney: you are free to delete the label | 16:08 |
sean-k-mooney | workflow -1, [WIP] and the new state | 16:08 |
sean-k-mooney | well i like the lable and dislike the gerrerit state | 16:08 |
clarkb | right so again, the way to address that is to provide the feedback upstream on why the gerrit state is not good | 16:09 |
sean-k-mooney | i find the native feature harder the ui and mange | 16:09 |
clarkb | and work to fix it. They are very receptive to that feedback and we are in a position where these things are actionable finally | 16:09 |
clarkb | in the past trying to solve every one of those minor issues is why we have fallen so far behind gerrit releases. With the size of the team now we've basically declared none of that is possible, we'll keep up with upstream and we can provide feedback to upstream on what works or doesn't work for us | 16:10 |
sean-k-mooney | clarkb: if they put the work in progress and draft options in the popup for the reply box it would annoy me less | 16:11 |
clarkb | so far we'ev been doing a decent job of that. And in some cases we're even pushnig fixes ourselves | 16:11 |
clarkb | But sitting on old gerrit and hoping we can keep status quo was no sustainable for us | 16:11 |
clarkb | It definitely isn't perfect though and our use cases are a bit different than google's but they truly are receptive to the feedback and I have been really happy with that | 16:12 |
fungi | especially once the versions we were running stopped getting security fixes | 16:12 |
sean-k-mooney | thats good to here re feedback | 16:12 |
*** amoralej is now known as amoralej|off | 16:30 | |
*** dviroel|lunch is now known as dviroel | 16:38 | |
*** ysandeep is now known as ysandeep|out | 17:30 | |
*** jpena is now known as jpena|off | 17:32 | |
*** tkajinam is now known as Guest1307 | 18:43 | |
*** dviroel is now known as dviroel|brb | 20:48 | |
opendevreview | Gage Hugo proposed openstack/project-config master: Retire security-specs repo - Step 3 https://review.opendev.org/c/openstack/project-config/+/827178 | 21:20 |
opendevreview | Gage Hugo proposed openstack/project-config master: Retire security-specs repo - Step 3 https://review.opendev.org/c/openstack/project-config/+/827178 | 21:33 |
opendevreview | Clark Boylan proposed openstack/project-config master: Set centos-8 min-ready to 0 https://review.opendev.org/c/openstack/project-config/+/827183 | 21:38 |
opendevreview | Clark Boylan proposed openstack/project-config master: Remove centos-8 https://review.opendev.org/c/openstack/project-config/+/827184 | 21:38 |
melwitt | I dunno if anyone else has noticed these kind of spam messages on launchpad https://bugs.launchpad.net/tempest/+bug/1959467/comments/2 https://bugs.launchpad.net/manila/+bug/1959472/comments/3 they're from the same user | 21:54 |
clarkb | neat. in the past they were super blatant but now they are trying to match general form for what discussion would look like | 21:55 |
clarkb | melwitt: I think if you report them to launchpad they kill the accounts | 21:55 |
melwitt | oh ok, will do that | 21:56 |
melwitt | found the reporting area at https://answers.launchpad.net/launchpad if anyone else was curious | 22:21 |
*** dasm|rover is now known as dasm|off | 22:29 | |
clarkb | thanks. I think people have also used IRC in the past for this sort of thing but ^ is nice because it leave a record | 22:32 |
fungi | yeah, #launchpad on libera is usually helpful | 22:51 |
melwitt | thanks | 22:52 |
*** ysandeep|out is now known as ysandeep | 23:30 | |
opendevreview | Ian Wienand proposed openstack/project-config master: Stop building CentOS 8 https://review.opendev.org/c/openstack/project-config/+/827195 | 23:42 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!