Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-09-08
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
17:00:04 sean-k-mooney those are thigns we can likely improve and im currently testing your fast8 fix
17:01:12 dansmith even if I can just `touch .do-not-fuck-up-my-code` and have that drive the in-place flag, that's fine
17:02:10 sean-k-mooney i need to look into how we can do that but ill see if we can supprot somethign ideally via an env var set by tox for you
17:03:20 sean-k-mooney i have never seen it actully break code in my own experince by the way so im sorry you have encotered that
17:06:05 sean-k-mooney dansmith: the fast8 fix works for me locally so i have approved it
17:06:32 sean-k-mooney sylvain was already +2
17:06:37 gibi dansmith: (with limited brain power left in me) you want to run pep8 locally but you dont want autopep8 to modify your code because you are in a middle of something. Still you are (not happily) accepting the fact that at the end before you push up the commit you will run autopep8 and let it do its things. If that is the case then totally supportive to your needs
17:07:11 gibi * then I totally
17:07:35 dansmith gibi: I do not ever want autopep8 to modify my stuff, even pre-push.. I want it to tell me what it objects to
17:07:47 sean-k-mooney for what its worth pre-commit ignored unstaged files but obviously that woudl be a change in your workflow jsut lettign you know
17:08:04 dansmith but yes, the major complaint is when I'm in the middle of a rebase or a complex change and it modifies my files and complicates my index so I have to manually revert things I don't want to change in the middle of a workflow
17:08:36 sean-k-mooney dansmith: that how i orgianlly implented it for what its worth autopep8 in the pep8 target complaied but did nto modify
17:08:42 sean-k-mooney in its sepreate target it fixed things
17:08:48 sean-k-mooney and via pre-commit it fixed things
17:08:50 dansmith sean-k-mooney: I know, that's what I want
17:09:22 dansmith since I'm not going to use pre-commit, that was fine.. and it was presented to us as fully optional because (a) it was a different tox target or (b) pre-commit was optional if you installed it
17:09:41 dansmith then, we moved to pep8 target using pre-commit and it's been complicating rebase workflows ever seince
17:09:41 gibi dansmith: yeah I should have be more specific. Either running autopep8 and let it do its things, or run autopep8 and fail if wants to modify something and you iterate manually until autopep8 is happy
17:09:53 dansmith gibi: yes, thanks
17:10:41 gibi dansmith: I'm OK with this approach. I will rely on autopep8, you will satisfy autopep8 manually. We won't clash and in CI the end result is the same happy autopep8 run
17:10:58 dansmith I had been working around this by using fast8 (which I usually use during a rebase anyway) and not complaining, but that broke recently as well hence the issue
17:11:08 dansmith gibi: ++
17:11:16 sean-k-mooney im trying to think how to do that today and i htink i might know

Earlier   Later