Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-10-02
09:08:18 opendevreview sean mooney proposed openstack/nova master: remove use of % format strings https://review.opendev.org/c/openstack/nova/+/896985
09:08:18 opendevreview sean mooney proposed openstack/nova master: pyupgrade py 3.0 -> 3.6 https://review.opendev.org/c/openstack/nova/+/896986
09:08:19 opendevreview sean mooney proposed openstack/nova master: add precommit pyupgrade hook and tox env https://review.opendev.org/c/openstack/nova/+/896987
09:08:19 opendevreview sean mooney proposed openstack/nova master: ignore pyupgrade series in git blame https://review.opendev.org/c/openstack/nova/+/896988
09:09:45 opendevreview sean mooney proposed openstack/nova master: ignore pyupgrade series in git blame https://review.opendev.org/c/openstack/nova/+/896988
09:10:22 opendevreview sean mooney proposed openstack/nova master: ignore pyupgrade series in git blame https://review.opendev.org/c/openstack/nova/+/896988
09:26:37 dvo-plv bauzas, good to hear,thank you
10:20:34 opendevreview Stephen Finucane proposed openstack/nova master: doc: Remove crud from conf.py file https://review.opendev.org/c/openstack/nova/+/896224
10:20:35 opendevreview Stephen Finucane proposed openstack/nova master: pre-commit: Update plugin versions https://review.opendev.org/c/openstack/nova/+/896226
10:20:35 opendevreview Stephen Finucane proposed openstack/nova master: tox: Use pre-commit for pep8 target https://review.opendev.org/c/openstack/nova/+/896227
10:20:36 opendevreview Stephen Finucane proposed openstack/nova master: pre-commit: Add mypy https://review.opendev.org/c/openstack/nova/+/896228
10:20:36 opendevreview Stephen Finucane proposed openstack/nova master: Blacken codebase https://review.opendev.org/c/openstack/nova/+/896229
10:20:37 opendevreview Stephen Finucane proposed openstack/nova master: pre-commit: Integrate black https://review.opendev.org/c/openstack/nova/+/896230
10:20:37 opendevreview Stephen Finucane proposed openstack/nova master: Ignore black change https://review.opendev.org/c/openstack/nova/+/896231
10:20:38 opendevreview Stephen Finucane proposed openstack/nova master: pre-commit: Use native hacking pre-commit hook https://review.opendev.org/c/openstack/nova/+/897005
12:38:34 opendevreview sean mooney proposed openstack/nova master: remove deprecated vmware virt driver https://review.opendev.org/c/openstack/nova/+/897017
12:55:30 opendevreview Merged openstack/os-vif stable/2023.2: Update TOX_CONSTRAINTS_FILE for stable/2023.2 https://review.opendev.org/c/openstack/os-vif/+/894062
14:56:47 stephenfin bauzas: I'm not sure I understand your comment on https://review.opendev.org/c/openstack/nova/+/896226/? Does my reply make sense?
14:57:41 bauzas stephenfin: tbc, I'm not a big fan of pre-commit hooks
14:57:59 bauzas they slow our development process and avoid new contributors to easily push some code asking for help
14:58:15 stephenfin but you don't need to use them
14:58:17 bauzas in general, I do atomic commits and then I squash
14:58:39 bauzas the more pre-commit hooks we add, the slower a git commit happens
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 :) )

Earlier   Later