Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2021-09-21
15:32:18 stephenfin bauzas: git commit -n
15:32:26 bauzas reminder : nova meeting in 28 mins-ish
15:33:08 bauzas clarkb: yeah that's one of my concerns
15:33:19 bauzas we push more and more checks into commit stage
15:33:28 bauzas and we add more linters
15:33:56 sean-k-mooney and we improve the quaity of the code and our developemt workflow as a result
15:34:27 stephenfin don't make humans do things that machines are good at
15:34:39 clarkb if tox and pre commit run the same checks they should run in about the same amount of time
15:35:15 sean-k-mooney clarkb: in some cases yes
15:35:15 stephenfin yeah, pre-commit and 'tox -e fast8' will run in ~ the same time, but you need to remember to do the latter
15:35:16 bauzas stephenfin: I don't disagree, I'm just on clarkb's side on the fact that could be left to tox
15:35:20 clarkb stephenfin: the problem is discoverablity. Humans don't know what all the checks are and some of them don't run until you get to zuul (commit message linting). Also some of it is severe nitpicking like having a '.' in the subject line or not
15:35:36 bauzas and linter jobs could be non-voting
15:35:53 stephenfin we have commit message linting?
15:35:57 bauzas we do
15:36:00 clarkb stephenfin: yes that is what started this entire conversation
15:36:02 sean-k-mooney bauzas: they could be but unless we are going to do that with pep8 im really not ok with that
15:36:02 bauzas for backport checks
15:36:04 stephenfin outside of the cherry-picked from line?
15:36:20 clarkb stephenfin: the bot that fixes your .gitreview file when you make a new branch got -1'd because its commit message wasn't good neough
15:36:31 sean-k-mooney clarkb: yep
15:36:45 sean-k-mooney the both should have had a stable only statement in that commit
15:36:54 clarkb the bot predates the check :P
15:37:12 sean-k-mooney i know but its not the first time we have modfied the bots patch
15:37:13 stephenfin clarkb: which change is this, specifically?
15:37:35 sean-k-mooney stephenfin: the one for the .gittreview file for the stable branch
15:37:39 stephenfin for nova?
15:37:42 sean-k-mooney yes
15:37:48 clarkb https://review.opendev.org/c/openstack/nova/+/809759
15:38:46 stephenfin okay, that's what I was expecting to see. Where's the check for the '.' in the subject line?
15:38:52 stephenfin Or is that another project?
15:39:24 clarkb stephenfin: that was an old checker for hacking. I bring it up because after much fighting we finally conceded that any checks on commit messages are a bad idea because nothing checks them until it is too late
15:39:40 stephenfin Ah, okay, thanks. I'm caught up now :)
15:39:41 clarkb basically any linter checks for commit messages are doomed to fail
15:39:48 clarkb because nothing runs the linters post commit
15:39:53 clarkb (except for zuul)
15:40:20 sean-k-mooney clarkb: by the way if i am runging tox its often after i commit
15:40:45 clarkb ok I don't know anyone else that does that unless they are bisecting to find issues in already merged code
15:40:55 stephenfin I agree that we should carve out an exception for these bot generated changes to stable
15:41:06 sean-k-mooney or just fix them
15:41:12 clarkb there is a semi related issue here with PBRs support for encoding semver requirements in commit messages
15:41:41 clarkb that causes a ton of problems too because commit messages will be merged and pbr will disagree with the version someone tagged by hand and then you end up i na really weird spot
15:42:02 clarkb (basically you need to be flexible with commit messages because you creat headaches when you try to enforce too many rules around them)
15:42:12 stephenfin sean-k-mooney: they've always been that way though so it seems weird to let our custom script dictate what the commits for every other project looks like
15:43:05 sean-k-mooney i semi agree but on the ohter hand hacking allows things i hate and blocks things i like yet we allow it to keep the porject semi consitent
15:43:14 lyarwood I've likely missed something here but why does this matter if a core still needs to ACK the bot proposed patches?
15:43:33 lyarwood and in ACK'ing these bot changes can add the [stable-only] tag?
15:43:36 sean-k-mooney stephenfin: when we added the script we were hoping ti would gain adoption in other porjects too
15:43:51 lyarwood it's not like these changes are merge automagically right?
15:43:55 lyarwood merged*
15:44:04 sean-k-mooney correct
15:44:18 sean-k-mooney i dont see why the stable team cant fix the bot patches when they approve
15:44:31 clarkb the whole point of having the bot is to reduce human overhead
15:44:34 stephenfin clarkb: Agreed in general. This feels slightly different though. It was put in because dansmith was bothered by me proposing backports to multiple branches at once, in the fear that they'd merge in the wrong order or a patch higher up would be modified and the changes wouldn't be captured in later patches. Those still seems like sensible concerns that'd be easily missed by reviewers
15:44:44 clarkb essentially we've got two bots that are in theory supposed to reduce overhead but in reality dobule it
15:45:03 lyarwood well that's on the stable team who want the second bot surely?
15:45:11 lyarwood I don't get the argument here tbh
15:45:31 lyarwood we want the cherry-pick job and are happy with the overhead
15:45:37 lyarwood surely that's enough?
15:45:47 clarkb I've got two main concerns. The first is that after many yaers we still act like proposing a non backport change to stable branches is immediately wrong. This is frustraing for people like me who basically only do that when touching stable branches
15:46:09 clarkb the second issue is that any linter applied to commit messages is problematic because you don't typically run linting post commit
15:46:10 sean-k-mooney im more or less of the opipion that we keep the cherry pick lines and enforce them in a job or we done enforce them in a job and dont require them any more
15:46:55 sean-k-mooney clarkb: well actully having the bot tell you its wrong to propose directly to stable was one of the goals of the script
15:47:12 sean-k-mooney clarkb: its not that uncommon to have one off patches propsoed directly to stable
15:48:07 sean-k-mooney i have -1'd patches a cople of time for that and explained that the issue need to be resolved in master first
15:48:39 elodilles clarkb: just for the record, this cherry-pick-check is only in nova repository (well, and afaik cyborg adopted it as well), all the other repositories are not having it
15:53:36 lyarwood clarkb: sorry back, I appreciate that the initial -1 is slightly off putting but again if the stable team looking after the repo are happy to do the work to fix this while they approve then I can't see an issue here.
15:53:47 opendevreview Stephen Finucane proposed openstack/nova master: tools: Ignore bot-generated branch creation patches https://review.opendev.org/c/openstack/nova/+/810285
15:54:04 stephenfin lyarwood, sean-k-mooney, elodilles: eh? ^
15:54:25 gibi stephenfin: thanks!
15:54:28 dansmith stephenfin: can we not just agree on some flag for bot-generated things?
15:54:28 stephenfin no need to backport that, obv. It'll just avoid this discussion come stable/yoga creation
15:54:34 lyarwood ha hackaroundFinucane has entered the chat
15:54:54 stephenfin dansmith: Sure. I just don't want to fix the bot :)
15:55:02 dansmith checking for those strings seems fragile, but I'll also say I agree with lyarwood and do not understand why humans can't just handle this
15:56:08 dansmith are the bots always the same userid?
15:56:26 dansmith i.e. checking for owner might just be easier
15:56:44 clarkb I think there are a couple of bots but it is a small number
15:57:46 bauzas reminder : 3 mins before the nova meeting
15:58:26 bauzas honestly, I stopped discussing this
15:58:55 dansmith I also think that assuming everything proposed against stable is wrong until proven right is the correct approach, just FYI
15:59:44 clarkb ok I won't worry about the bot being told it is wrong then
15:59:57 opendevreview Stephen Finucane proposed openstack/nova master: tools: Ignore bot-generated branch creation patches https://review.opendev.org/c/openstack/nova/+/810285
15:59:59 stephenfin dansmith: Good idea ^
16:00:23 dansmith stephenfin: ++
16:00:51 opendevmeet The meeting name has been set to 'nova'
16:00:51 opendevmeet Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
16:00:51 opendevmeet Meeting started Tue Sep 21 16:00:51 2021 UTC and is due to finish in 60 minutes. The chair is bauzas. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:00:51 bauzas #startmeeting nova
16:01:14 bauzas #link https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting
16:01:28 bauzas sorry for interupting you, folks :)
16:01:37 bauzas who's around ?
16:01:45 dansmith o/
16:01:49 lyarwood \o
16:01:53 elodilles o/
16:02:28 gibi o/
16:02:55 bauzas we have some large discussions today, let's start quickly
16:03:04 bauzas #topic Bugs (stuck/critical)
16:03:08 bauzas No Critical bug
16:03:14 bauzas #link 13 new untriaged bugs (-0 since the last meeting): #link https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New

Earlier   Later