Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-10-02
14:58:45 stephenfin only if you enable them
14:58:54 stephenfin which I'm guessing you don't?
14:58:56 bauzas sure, but you're adding them by default
14:59:13 bauzas which relates to my other concern
14:59:14 stephenfin I am?
14:59:23 bauzas and yeah, I know I can disable hooks
14:59:37 stephenfin hooks aren't enabled by default. It's a manual step to install them
14:59:45 bauzas that's why I'm proposing an half-way solution, which is to use pre-push hooks
15:00:07 bauzas it's an automatic step on my env
15:00:28 stephenfin before we talk about that, back up: where is this being added by default?
15:00:52 stephenfin if you don't run 'pre-commit install' the hooks are irrelevant
15:03:04 bauzas I need then to check something
15:03:17 bauzas because I'm sure it's pulling pre-commit without my request
15:04:03 stephenfin I'm very confused. Do you have pre-commit installed? If not, it isn't capable of installing itself or anything like that
15:05:28 bauzas I do
15:05:59 stephenfin Okay, if it's annoying couldn't you just remove it? The CI will still catch things for you
15:06:03 bauzas and yeah I wonder how that package went into my env
15:06:34 bauzas stephenfin: I surely can remove that package
15:07:34 bauzas stephenfin: can we actually discuss that topic into our PTG sessions if you can attend ? black is a different matter, but I'd like us to agree on what are the expectations of a git commit
15:07:51 bauzas I'm not sure everyone talks the same language
15:11:14 stephenfin We already have expectations of a git commit. The hard ones are encoded in the pep8 job. Nothing has changed
15:12:40 stephenfin I can understand the merit of discussing the black stuff there, but pre-commit is simply a wrapper around tooling that we've already agreed on (autopep8, hacking, mypy). Do we need 20+ people discussing a tool like that?
15:14:18 clarkb did precommit fix the problem of not installing tools from pypi?
15:15:28 stephenfin That'd be a non-goal since PyPI is a Python-specific repository and pre-commit is intentionally language agnostic
15:15:55 stephenfin at least, I imagine that to be the case? tbh I've never looked :)
15:18:13 bauzas I just think that not a lot of people exactly understand which tools are run and why
15:18:29 bauzas and if they see a pre-commit YAML file, they'll pull it
15:19:41 bauzas but if we are clear on the line that pre-commit is optional, then I question the need
15:19:47 sean-k-mooney bauzas: they way we use pre-commit now its entirly optional to install the hook locally and all automations are enforced in the tox jobs already
15:19:51 bauzas for our CI, I guess ?
15:20:32 bauzas sean-k-mooney: the way it's done is that *if* you have pre-commit installed, then you're tied to its rules
15:21:02 sean-k-mooney yes but the pep8 job aslo runs the same checks
15:21:17 sean-k-mooney as does tox -e pep8
15:22:16 bauzas sean-k-mooney: sure, but then I question the benefits
15:22:40 sean-k-mooney its faster for one
15:22:45 sean-k-mooney vs what we are doing today
15:23:11 sean-k-mooney you can also skip pre-commit whenever you want with --no-verify
15:23:19 sean-k-mooney if you have it installed
15:23:24 clarkb how is it faster if it runs the same checks?
15:23:28 sean-k-mooney if you dont have it installed the you can run it with tox
15:23:36 sean-k-mooney and not install it or change your workflow
15:23:52 sean-k-mooney clarkb: by default its flake8 check just the files you modifed
15:24:12 sean-k-mooney so its the same as our old fast8 tox env
15:24:29 sean-k-mooney for other things its not neessisarly any faster
15:24:35 dansmith it's still faaar too slow for run-on-commit for me :)
15:24:47 bauzas I just feel we absolutely need to amend https://docs.openstack.org/nova/latest/contributor/development-environment.html#using-the-pre-commit-hook
15:24:54 bauzas to make it optional
15:25:00 sean-k-mooney it is optional
15:25:03 bauzas and specify how to skip it
15:25:13 bauzas sean-k-mooney: our docs aren't that clear
15:25:27 sean-k-mooney if you dont install it then it does not run
15:25:32 sean-k-mooney if you do its just --no-verify
15:25:40 sean-k-mooney but we can updated it to be more clear
15:25:47 melwitt you can also just do -n
15:26:06 bauzas I think I can and shall update that docv
15:26:33 melwitt and the doc was clear enough to me, fwiw. i.e. I understood pre-commit doesn't happen unless you install the pre-commit package
15:26:34 sean-k-mooney melwitt: oh good to know
15:26:48 sean-k-mooney yep "This must be enabled locally to function:"
15:26:53 sean-k-mooney was ment to capture that in the doc
15:27:37 bauzas then I'll remove my vote on the pre-commit additions change
15:27:46 melwitt (I'm not objecting to improving the doc btw, just saying at least to me I've used the doc and it made sense for me)
15:28:18 sean-k-mooney bauzas: for what its worht i have other related but seperate changes
15:28:54 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/896987
15:32:45 sean-k-mooney i was going to add a "tech-debt reduction and code modernization" topic to the ptg (items for this seession would by hyperv and vmware driver removal. black and pyupgrade)
15:34:13 sean-k-mooney we have blueprints filed for at least 3 of those. https://blueprints.launchpad.net/nova/+spec/remove-hyperv-driver https://blueprints.launchpad.net/nova/+spec/pyupgrade and https://blueprints.launchpad.net/nova/+spec/remove-vmware-support
15:34:47 sean-k-mooney i dont know if we have or need one for automated formating but we can at least have the conversation at the ptg
15:35:00 bauzas sean-k-mooney: I'd be in favor of more fine-scoped topic names
15:35:20 stephenfin bauzas: Thanks. Good to hear that confusion is cleared up (y)
15:35:22 dansmith I'm prepared to die on black hill
15:35:38 bauzas at least for clarifying the intent and clearly timekeep on subjects
15:36:23 bauzas one topic I could add could be "volume attachment improvements" but that's exactly kind of the same vague bucket :)
15:36:38 sean-k-mooney i wanted to group related topics
15:36:43 sean-k-mooney hence the sub items
15:37:08 bauzas sean-k-mooney: if you don't mind, stick with items per each, and I'll just make sure we loop thru them as a group
15:37:12 stephenfin dansmith: You'll be happy to hear sean-k-mooney also does not like the spreading of arguments across multiple lines 0:) Apparently there's some way to change that but I didn't look into it yet
15:37:16 sean-k-mooney those 3 are all tech debth reduction and as an fyi i have fully implemted pyupgrade adn vmware removal
15:38:04 bauzas (fwiw, the vmware one seems to me quite non-controversial :) )
15:38:12 sean-k-mooney i have one open question on it
15:38:25 sean-k-mooney baiscally i intentionl didn not make any api changes but i proably should
15:38:32 bauzas if you stopped a dead corner, then sure :)
15:38:40 sean-k-mooney i.e. i did not remove the mks console server or any of the extra specs validation ectra
15:38:43 bauzas spotted*
15:38:53 sean-k-mooney but want to hear how people want that to be done
15:38:55 bauzas yeah, I thought it was API-related
15:39:05 sean-k-mooney the currnt patch is not api related
15:39:15 bauzas we have a solid experience on API stability while we remove backends
15:39:23 sean-k-mooney just the virt driver and remvoe teh virt dirver spcifric cofnig options and docs.
15:39:48 sean-k-mooney i belive gman has also already done hyperv and has the same open
15:39:58 sean-k-mooney i.e. waht to do with hyperv specific api aspect
15:40:08 sean-k-mooney so i wanted to group does disucssions togetehr
15:40:13 sean-k-mooney since hte approch should likely be the same
15:41:05 bauzas sounds a good idea to me
15:41:17 bauzas thanks for catching it
15:41:34 sean-k-mooney do we have the ptg etherpad yet
15:42:23 bauzas yep
15:42:33 bauzas oh but you weren't in the last meeting, sec
15:42:47 sean-k-mooney ya still catching up form PTO
15:42:54 bauzas np
15:43:25 bauzas https://etherpad.opendev.org/p/nova-caracal-ptg
15:43:42 bauzas (didn't made any huge effort for the naming)

Earlier   Later