Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-09-08
14:48:11 bauzas the agreement being that autopep8 is and should always be optional
14:48:23 bauzas including in the tox target
14:48:31 dansmith bauzas: +1000
14:48:52 bauzas also, skipping pre-commit is unfair, as I need pre-commit for adding the gerrit change ID automatically, I can't just skip autopep8
14:49:15 bauzas (AFAIK, no way to only skip a subset of the multiple pre-commit hooks)
14:49:51 Uggla hum it is the case for quite some time right ?
14:50:34 dansmith Uggla: it has been yes and I've hated it this whole time, but this morning it's really killing me and I want it to stop
14:50:56 dansmith I also argued against it when it was even proposed before it was autopep8'ing
14:51:14 dansmith IIRC it used to just show the suggestions but not force them on me
14:59:12 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
14:59:18 Uggla it depends some changes are left to the author, but some basic reformatting are applied, if I'm not wrong.
14:59:44 dansmith yes and I never ever want it to do that..
15:00:14 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
15:00:18 dansmith if I have uncommitted changes and I run pep8, it will make a bunch of extra changes that I did not want that are all "opinions" of the tool and now I have more change queued
15:01:44 dansmith right now it keeps forcing this on me: 3
15:01:49 dansmith https://paste.opendev.org/show/bjPrI7fQi2rx3TH1ndDB/
15:02:03 dansmith which takes the code from readable to stupid for no effing reason
16:10:55 ratailor__ bauzas, melwitt can I get reviews on backports https://review.opendev.org/c/openstack/nova/+/958834 and https://review.opendev.org/q/project:openstack/nova+branch:stable/2024.2+owner:ratailor@redhat.com
16:12:15 opendevreview Takashi Kajinami proposed openstack/nova stable/2024.1: libvirt: Disable VMCoreInfo device for SEV-encrypted instances https://review.opendev.org/c/openstack/nova/+/959914
16:20:06 sean-k-mooney bauzas: the change-id hook is entirly seperate
16:21:19 sean-k-mooney you can 1 not install pre-commit, 2 if you have it install do --no-verify to dsiabll all hooks other then the change id hook ro 3 use SKIP=list,of,hook.ids as some options
16:21:30 bauzas sean-k-mooney: sorry, I meant that --no-verify skips all the hooks
16:21:56 sean-k-mooney all the hooks form the pre-commit tool but i dont think it skips the change id hook
16:22:50 sean-k-mooney dansmith: that may be a side effect of a change i did a while a go to make autopep8 fix more things
16:22:56 sean-k-mooney we coudl look at trunign that off
16:23:07 dansmith please please please turn it off
16:23:15 bauzas sean-k-mooney: the git help says that it also skims commit hooks, so the gerrit one
16:23:24 sean-k-mooney ack
16:23:36 bauzas commit-msg sorry
16:23:37 sean-k-mooney let me find the comit and pose a revert and see if that help
16:24:41 sean-k-mooney i basically took some of the config form oslo/sdk and ported it a few monts ago
16:25:05 dansmith fast8 also was broken, I assume by one of these changes a while ago which is also really frustrating
16:25:33 dansmith I haven't tried fixing it yet, but it's really annoying because it's like "please try pre-commit instead!" with the implication of "so I can break your code for you"
16:25:41 sean-k-mooney well that was ment to bedeelted a while ago but i dont think that is related
16:26:20 dansmith I dunno why it was "meant to be deleted" .. I assume that's just your opinion, but it's a huge time saver
16:27:15 sean-k-mooney so this https://github.com/openstack/nova/commit/ebd75760e1907bb4b1bc089ecf6f4405bf52e208 might be the issue
16:27:44 sean-k-mooney dansmith: not just that it has not been maintaiend for a while an i otught it broke with some of the ohter seuptools change ectra
16:27:57 sean-k-mooney i dont reacall honestly because i have not used it in a few years
16:28:22 dansmith I will propose the revert for that if you don't
16:29:02 sean-k-mooney aggressive = 3 is the actual change in behavior but when we did change it it dint really change mouch code
16:29:09 sean-k-mooney but sure if tis cause issue swe can drop that
16:31:25 sean-k-mooney ok i need to do that form the cli...
16:33:23 tkajinam or you can just copy-paste the string
16:33:30 tkajinam it does not require actual signing
16:33:35 dansmith Uggla: I'd really like to get wider team agreement on more things like this: https://review.opendev.org/c/openstack/nova/+/949292 before they merge
16:33:48 sean-k-mooney tried that but i cant use my normal email if i do that
16:34:00 tkajinam ah, yeah
16:34:15 sean-k-mooney i could swap to my redhat one but it complain about the commit not matchign the sgined off by line since my redhat email is my gerrit default
16:34:33 sean-k-mooney it woudl be better if it allwoed any of them or allowed my to select like if im editing things in the ui
16:34:51 tkajinam I remember I faced similar problems when I tried to rebase your change.
16:34:52 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
16:35:11 tkajinam so yeah I agree with you
16:35:50 opendevreview Balazs Gibizer proposed openstack/nova master: Rally job for eventlet-removal https://review.opendev.org/c/openstack/nova/+/960130
16:36:01 opendevreview Dan Smith proposed openstack/nova master: Fix fast8 tox target https://review.opendev.org/c/openstack/nova/+/960149
16:36:10 dansmith bauzas: ^ can haz please to fix fast8
16:36:14 dansmith it's trivial
16:36:27 opendevreview sean mooney proposed openstack/nova master: Revert "Allow autopep8 to fix more things" https://review.opendev.org/c/openstack/nova/+/960150
16:37:01 dansmith bauzas: and also that ^
16:37:39 sean-k-mooney dansmith: can you test that locally by remving the agressive=3 line
16:37:50 sean-k-mooney that actully the only thing it is really changing
16:38:07 sean-k-mooney i think the default is 0 or 1
16:40:54 dansmith I don't ever want in-place=True, assuming that's the permission to modify anything
16:41:19 sean-k-mooney yes that what that enables
16:41:55 dansmith removing aggressive=3 still modifies my file
16:41:55 sean-k-mooney when invoked form per-commit it will do that by defualt adn recurse
16:42:04 dansmith yeah -2 on that
16:42:19 sean-k-mooney well we have been doing that and agreed to do that for years
16:42:24 sean-k-mooney that was not part of this change
16:42:36 sean-k-mooney i jsut encoded the defaults we already had implictly
16:42:48 dansmith we have not "agreed to do that for years"
16:42:58 sean-k-mooney well it not new
16:43:10 sean-k-mooney it was not part of that change
16:44:35 dansmith I do not agree with the fact that we're using pre-commit for this stuff, and you and others sold it as "dude it's totally optional if you just don't install this"
16:45:06 dansmith well this sort of modifying of my stuff while I'm working is not okay with me and it's been a nuisance this whole time but today it really blew up my whole morning
16:45:17 sean-k-mooney when autopep8 was added it was entirly optional
16:45:22 dansmith I do not think it's unreasonable to have a target you can run if you want to autopep8 your code
16:45:38 dansmith I do not want that
16:45:56 dansmith sean-k-mooney: right and now I can't even run the pep8 target (even with your revert applied) without it making changes to my code
16:46:08 dansmith removing the aggressive line makes those changes much less stupid, but it still does it
16:46:09 sean-k-mooney right becaus that was changed by https://github.com/openstack/nova/commit/2ffdf2e9173ca2ad0f95ac7f565da64b8b62691f
16:46:53 sean-k-mooney when we swap to runign pep via per-commit we got precommits default of formatign the code by default
16:47:12 sean-k-mooney this has been the case since december 2023
16:47:24 dansmith what is your point?
16:47:47 sean-k-mooney 1 other want the changed behvior, 2 i didnt actully change it and 3 its not new
16:48:34 sean-k-mooney so please dont shot the messanger im jsut providing you the lines to the changes that modifed the behvior
16:48:35 dansmith I understand this has been modifying my code against my will since end of 2023.. I'm saying I've been hating it since then and as of this morning I've been tolerating it because the changes have been minor
16:50:46 dansmith I think it's very reasonable to have pre-commit and the pep8 target not modify anything but just warn, and if people want to run autopep8 on their code, we can have a tox target to do so
16:50:49 sean-k-mooney to be clear the fact this has signifcantly reduced the tiem we talk about formating has been a major quality of life improvment for me in general. we coudl revert to not using pre-commt in pep8 targe tbut that was also a sginficat performace win
16:51:19 dansmith sean-k-mooney: I understand that you feel that way, but I do not
16:52:19 dansmith you asserting that we as a team agreed on something you wanted when it was merged five days before christmas when maaaany people are on PTO is a major quality of life non-improvement for me
16:52:25 dansmith so it'd be helpful if you could stop doing that
16:54:18 sean-k-mooney the change was up for review for several months
16:54:42 sean-k-mooney im not assertign that because i wanted it
16:54:51 sean-k-mooney just that it wen through our normal review process
16:55:40 sean-k-mooney anywya i dont think this is a useful confveration to have now but if you want to have it when more are aroudn thne do
16:55:56 sean-k-mooney we can alwasy bring it up in an irc meetign or at the ptg
16:58:16 dansmith gibi: I know you're pro-formatting.. can you understand that I want to be able to pre-pep8 my code before submitting without autopep8 modifying my local index, especially given this morning it was breaking it pretty badly?
16:58:29 dansmith at least enough to have two targets, one of which just runs without modifying?
16:59:01 dansmith if more people want the default target to run modify mode then fine, but I want to be able to run a linter in read-only mode
16:59:23 dansmith it kills me that I can't git rebase -x to check without it interrupting me

Earlier   Later