Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-09-21
13:24:35 bauzas lyarwood: if you are interested in this role, tell me
13:24:47 gibi this reminds me to book a dentis checkup too
13:25:02 bauzas gibi: booked since 6 months
13:25:18 gibi I'm not rushing :)
13:25:29 kashyap gibi: Yikes; I've been avoiding it
13:25:35 kashyap I know I should
13:26:33 bauzas "I have a tooth rage", "sure, we have a slot in Feb 2022, works for you ?"
13:26:47 bauzas longer than delivering a car
13:36:56 gibi as far as I see I can book a dentist slot for this thursday morning
13:37:05 gibi that feels too close :/
13:40:01 kashyap gibi: Don't you prefer your band-aid to be quickly removed?
13:40:25 gibi that one way to look at it
13:41:07 artom_ bauzas, obviously the solution is to drive a new car over your tooth
13:41:09 artom_ No tooth, no pain
13:41:27 bauzas heh
13:42:17 gibi artom: :D
13:52:22 bauzas gibi: agenda is updated https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting feel free to add your points
13:52:38 bauzas gibi: will you propose a backport to stable/xena for the decorator bump ?
13:52:49 gibi bauzas: ack will check the agenda
13:53:24 gibi bauzas: I can propose a backport then we can drop it if we choose other option to go forward
13:56:11 opendevreview Balazs Gibizer proposed openstack/placement stable/xena: Bump min decorator to 4.0.0 https://review.opendev.org/c/openstack/placement/+/810193
13:56:16 gibi bauzas: done
13:56:28 bauzas thanks
13:56:40 bauzas we could discuss this during the meeting then
14:02:57 gibi added two topic to the end of the agenda
14:20:04 opendevreview Thomas Goirand proposed openstack/nova stable/xena: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/810192
14:21:28 zigo gibi: bauzas: My bad, I shouldn't have pushed an update, now this patch needs another +2W: https://review.opendev.org/c/openstack/nova/+/810192
14:21:58 zigo I was waiting for the .gitreview, and thought I should be doing it...
14:33:35 opendevreview Elod Illes proposed openstack/nova stable/xena: Add missing __init__.py in nova/db/api https://review.opendev.org/c/openstack/nova/+/810192
14:34:47 elodilles zigo: i've uploaded the original patch again
14:35:35 zigo elodilles: Thanks.
14:39:44 elodilles zigo: and +2+W'd as the content is now the very same as PS1 had
14:48:40 clarkb gibi: thats kind of my point though updating things like tox.ini or .gitreview or translations or other mechanical infrastructure bits don't need that and aren't really a corner case. The lint rules are just too aggressive and you can trust reviewers more imo
14:48:59 clarkb It is ok to update the branch so that it functions
14:49:33 clarkb but if you insist on having those rules please update the bots so that they don't run afoul of the linter
14:52:39 gibi clarkb: I'm not the right person to convince as I wasnt the one pushed for the stable backport linter. lyarwood, elodilles: what is your view?
14:53:43 gibi clarkb: I do think that having couple of paches needing a manual touch every six months is not a big deal. But I also see the point that we could update the bot to use a specific commit message in nova stable setup patches
14:53:58 clarkb I probably feel more strongly about this than most because 90% of the time if I'm pushing to stable branches it is to fix a stable branch specific issue without a backport and I always run into problems like this and it is frustrating that no one seems to accept it is ok to fix a branch on its own
14:54:07 bauzas gibi: oh shit, I wanted to discuss about the release liaison role but I forgot to add it in the meeting notes :facepalm:
14:54:24 gibi bauzas: I added it :D
14:54:57 bauzas I sqaw
14:54:59 clarkb if the branch is broken and someone pushes a fix reviewers should have the discretion to land the fix
14:55:12 gibi clarkb: thank you for taking case of the stable branches. I hope elodilles and lyarwood can join.
14:55:21 gibi ... and add their views
14:57:01 gibi s/case/care/
14:58:07 gibi clarkb: most of the linters are to help the reviews not missing things. in this case this linter helps the reviewer to check that the fix is already landed on master and newer stable branches.
14:58:20 bauzas honeslty I tend to prefer trusting reviews rather than using linters
14:58:26 gibi which is the rule, with exceptions
14:58:50 bauzas but I think I already said that a lot (context: mypy)
14:59:05 elodilles well, the point with that extra check is to ensure we have the *correct* patch backported
14:59:15 bauzas if linters block us instead of helping us, then it's a problem
15:00:27 elodilles as it sometimes happened that a patch was backported to several branch at the same time and later on modified (a middle patch) and those changes were not backported to older branches
15:00:28 gibi bauzas: what if linters both help us but sometimes blocking us
15:01:21 elodilles bauzas: don't look at that linter as a blocker but as a helper to catch human failure :)
15:04:11 elodilles about the .gitreview patch needed manual change: IIRC we had a prompt discussion in the past and decided that it's not a big deal to update those 2 patches every six month
15:07:22 clarkb elodilles: right I get that, the issue is occasionally (and usually when I'm touching stable) we have to modify the mechanics of the branch directly without a backport
15:07:49 clarkb this is an example of that case. I think reviewers can see that an update to .gitreview to set the correct branch or update a git review option is fine without a backport
15:10:49 elodilles clarkb: actually i also do stable fixes that are stable only and for those cases we (nova (and some other teams)) use 'stable-only' tags in commit message
15:11:27 clarkb elodilles: yes but that isn't consistent and you never updated the bot to do that (which is really my complaint)
15:12:10 clarkb you should disable the bot on your repos or update the bot
15:12:23 elodilles clarkb: ok, i see your point
15:12:29 clarkb personally I'd remove the lint rule entirely because the only way to discover the rule is to push code and have it fail first
15:12:30 elodilles clarkb: I'll update the bot
15:14:23 elodilles clarkb: well, in our team stable cores decided that this linter is useful for us (especially for reviewers). I understand your point, but i tend to look it from another perspective :)
15:17:17 clarkb ok. In general I think commit message linters are a bad idea. Users don't know they will fail until after they have pushed (because typical workflow is make changes, run tox, commit, push)
15:19:05 sean-k-mooney clarkb: we tried to strick a blance by having it non votign in check to give early feedback and block it only in gate
15:19:06 elodilles clarkb: without the linter we did the very same: if a stable reviewer saw that something is not complete in the commit message (or a wrong patch was backported) then we asked the author to change the patch
15:19:44 sean-k-mooney they can also run the lint check locally
15:19:45 clarkb elodilles: right but in the case of fixing the mechanics of a stable branch you wouldn't do that because it is fine as is
15:19:45 gibi clarkb: I think it is not impossible to add the current check as a tox target too (or part of pep8) so the extra push can be avioded
15:19:50 elodilles clarkb: and that linter (script) can be also run locally
15:19:51 sean-k-mooney most user do not backport
15:19:56 clarkb sean-k-mooney: they can but literally no one expects they have to run the linter after committing
15:20:07 clarkb this is why we removed the '.' at end of subject lines rule from hacking
15:20:28 sean-k-mooney clarkb: well we have been trying to encurage people to use precommit more by the way
15:20:37 clarkb oh wow. I would -2 that too :P
15:20:39 elodilles gibi: we have it as an extra tox target now as stephenfin proposed it a couple of months ago
15:20:49 clarkb (it doesn't handle dependencies properly and you'll completely bypass mirrors etc)
15:21:09 sean-k-mooney clarkb: pre comiit is not a repleace fdor any of our ci
15:21:24 sean-k-mooney it just elimnates the excuse fo forgetting to run expected checks
15:21:24 clarkb sean-k-mooney: right but people run it locally and may have proxies or mirrors etc
15:21:51 clarkb sean-k-mooney: the pre commit hook won't catch the commit issue errors though because it happens too soon right?
15:22:13 sean-k-mooney in this case yes
15:22:24 sean-k-mooney but it will run flake8 exctra
15:22:42 sean-k-mooney it is too early for this backport check
15:22:53 sean-k-mooney althogh technially
15:22:59 sean-k-mooney you can configure it to run on push
15:23:03 sean-k-mooney not on commit locally
15:23:15 clarkb it would be better to run it after the commit like the gerrit chaneg id hook
15:23:30 clarkb then people won't ask me why push is so slow to gerrit
15:23:53 sean-k-mooney yes so if you configure it to run pre-commit on push instead of commit then you get that effect
15:23:59 sean-k-mooney but its not the default
15:24:07 sean-k-mooney and i dont think they have a post commit option
15:24:43 sean-k-mooney clarkb: being able to run it on push instead of commit i think is relitively new to the tool
15:26:21 sean-k-mooney https://pre-commit.com/#top_level-default_stages
15:27:31 sean-k-mooney actully you might be able to add addtion stages https://pre-commit.com/#config-stages
15:27:45 sean-k-mooney this is beyond my basic use of the tool to date
15:28:07 bauzas sean-k-mooney: I'm not super happy either with pre-commit hooks
15:28:29 bauzas sometimes I just wanna push
15:28:38 sean-k-mooney bauzas: it removes a lot of the teedium with doing dev

Earlier   Later