opendevreview | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/896175 | 03:20 |
---|---|---|
bauzas | fyi, folks, did the launchpad usual janitory for creating a new series and milestones on both placement and nova : https://launchpad.net/placement/2024.1 and https://launchpad.net/nova/2024.1 | 09:07 |
bauzas | as you can see, I stopped using the release name to mock what we already have in our specs repo | 09:08 |
opendevreview | Merged openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/896175 | 09:36 |
stephenfin | bauzas: Can we merge the outstanding sqlalchemy 2.0 stuff now? | 10:11 |
stephenfin | actually, it's sean-k-mooney I should be asking. Sean, could you look at https://review.opendev.org/c/openstack/nova/+/860829/ again? | 10:12 |
sean-k-mooney | yes please | 10:13 |
stephenfin | though I am going to respin https://review.opendev.org/c/openstack/nova/+/886230 to address those regex warnings | 10:13 |
sean-k-mooney | what about https://review.opendev.org/c/openstack/nova/+/894635 | 10:15 |
opendevreview | Stephen Finucane proposed openstack/nova master: Add job to test with SQLAlchemy master (2.x) https://review.opendev.org/c/openstack/nova/+/886230 | 10:16 |
stephenfin | sean-k-mooney, bauzas: ^ fixed to address the RE2 warning from zuul. It's otherwise unchanged. | 10:16 |
stephenfin | sean-k-mooney: I'll clean that last patch up today | 10:16 |
stephenfin | actually, intermittent failure. That looks good to me now but I think bauzas wanted a little more proof that the relationships weren't used anywhere | 10:17 |
stephenfin | trying to figure out how I can prove that besides code inspection (I tried that. Too slow) | 10:17 |
sean-k-mooney | i didnt think it was related and likely just needed a retry | 10:24 |
sean-k-mooney | i was more wondifning if you still wanted to proceed with that patch | 10:24 |
sean-k-mooney | the first two in teh series are +2w ill review the ci patch shortly | 10:24 |
opendevreview | Felix Huettner proposed openstack/nova master: remove calls used for nova-network https://review.opendev.org/c/openstack/nova/+/896221 | 11:00 |
opendevreview | Stephen Finucane proposed openstack/nova master: doc: Remove crud from conf.py file https://review.opendev.org/c/openstack/nova/+/896224 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: Apply latest version of autopep8 https://review.opendev.org/c/openstack/nova/+/896225 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: pre-commit: Update plugin versions https://review.opendev.org/c/openstack/nova/+/896226 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: tox: Use pre-commit for pep8 target https://review.opendev.org/c/openstack/nova/+/896227 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: pre-commit: Add mypy https://review.opendev.org/c/openstack/nova/+/896228 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: Blacken codebase https://review.opendev.org/c/openstack/nova/+/896229 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: pre-commit: Integrate black https://review.opendev.org/c/openstack/nova/+/896230 | 11:55 |
opendevreview | Stephen Finucane proposed openstack/nova master: Ignore black change https://review.opendev.org/c/openstack/nova/+/896231 | 11:55 |
stephenfin | sean-k-mooney: ^ | 11:55 |
sean-k-mooney | :) | 11:55 |
stephenfin | That's what I was talking about. We just disable E501 because it's not worth the hassle of manually fixing the dozen or so exceptions | 11:55 |
stephenfin | It also avoids questions from contributors less familiar with the tooling. We can just tell them "run this and you'll get no style-related questions" | 11:56 |
sean-k-mooney | ya if other are ok with that then i would be infavor of the general useablity improvement | 11:56 |
stephenfin | (as opposed to "run this and then manually fix things if you still get errors") | 11:56 |
sean-k-mooney | although the eiarlier patches i htink should be done regradless | 11:56 |
sean-k-mooney | so ill start reveiwign those | 11:56 |
stephenfin | yup | 11:56 |
stephenfin | I put those at the front since they should be non-controversial | 11:57 |
frickler | while you're doing linting stuff, please also check https://review.opendev.org/c/openstack/nova/+/874517 , it seems to keep failing on unrelated issues | 12:04 |
sean-k-mooney | some of that might get adresses by these patches as well but sure | 12:04 |
sean-k-mooney | ya the bump to the new autopep8 fixes many of the hacking issues | 12:05 |
stephenfin | oh yeah, there's quite a bit of overlap there. The only thing mine are missing is the switch from printf style formatting (%) to use of string.format. I wonder why I didn't have to worry about that? | 12:05 |
sean-k-mooney | its not a pep8 requriement | 12:06 |
sean-k-mooney | and i would honestly prefer to go to fstring if we were updating them | 12:06 |
frickler | o.k., if that's no longer needed, just updating https://review.opendev.org/c/openstack/hacking/+/874516 would also be fine to me | 12:06 |
frickler | and once that's done we can do the next flake8 bump in hacking ;) | 12:06 |
bauzas | stephenfin: sorry was outside sweating | 12:07 |
bauzas | stephenfin: sure, if we want to merge your series, I prefer to do this on the starting Caracal :) | 12:07 |
sean-k-mooney | frickler: so stephen's partches are largely generated form tools | 12:11 |
stephenfin | bauzas: tbf, we say that every cycle and then we forget to look at it until the end of the cycle (see all discussions on the outstanding sqlalchemy 2.x work from last week) 0:) | 12:11 |
bauzas | stephenfin: that's why this is fresh in my mind | 12:11 |
stephenfin | the changes (except for the black one) are minimal and we've cut the branch, can we review now and if we need to fast revert, we can | 12:11 |
sean-k-mooney | so we will just proceed with the hacking fix patch and then regeenrate the ones that are still needed form there pre-commit series | 12:11 |
stephenfin | it'd likely be a win for you too: if we operated this way, I'd have stopped annoying you about sqlalchemy 2.x ~6 months ago XD | 12:12 |
stephenfin | sean-k-mooney: wfm | 12:12 |
sean-k-mooney | RC1 and stable/2023.2 have been done now | 12:13 |
sean-k-mooney | so i think now is a perfect time to do linting updates to be honest | 12:13 |
bauzas | can we try to not mix linter changes *and* SQLA things at the same time ? | 12:16 |
sean-k-mooney | they are two diffent serise | 12:16 |
bauzas | I know | 12:16 |
bauzas | but you know my backportability concerns about any linting change | 12:16 |
sean-k-mooney | yep and you knwo i disagree with that stance. that said the early one should have no impact | 12:17 |
bauzas | those need to be reasonabily needing a reason, because it makes any other backports a pain | 12:17 |
bauzas | we suffered a lot with functional tests refactoring, but this was kinda understansable | 12:17 |
bauzas | since it was improving tests reusability | 12:18 |
stephenfin | bauzas: out of curiosity, could you point to a recent case where a backport was impeded because of one of these linting change? | 12:18 |
sean-k-mooney | honetsly not automating this makes it harder for use to maintain nova | 12:18 |
sean-k-mooney | and it makes it harder for new peopel to continbue. with the reduced upstream comunity i think it would be in our intereste to embrance this automation where we can | 12:19 |
bauzas | stephenfin: I don't have any examples of the linting changes in mind, but be sure we faced some problems for other bits | 12:19 |
bauzas | sean-k-mooney: I understand that point for refactoring things | 12:19 |
bauzas | sean-k-mooney: but for linting, I seriously argue | 12:19 |
sean-k-mooney | yep and ill happy argue back | 12:20 |
bauzas | code style modifications being even less convincing to me | 12:20 |
sean-k-mooney | honestly i have spend enough time this cycle on sytle issues in code review | 12:20 |
sean-k-mooney | that if this is the only thing we got done in caracal i woudl consider it a success | 12:21 |
bauzas | I know it's Friday, but is this a reason to argue now ? | 12:21 |
sean-k-mooney | i mean its friday what else would we do | 12:22 |
stephenfin | :) | 12:22 |
sean-k-mooney | go home early and realx | 12:22 |
bauzas | I already sweated and now I have endorphins | 12:24 |
bauzas | not sure my stress level should jump, not good for my heart | 12:24 |
sean-k-mooney | if the wether stays ok i might go for a bike ride later | 12:24 |
sean-k-mooney | althogh its a bit humid out so we will see | 12:24 |
bauzas | seriously, I'm half-kidding, but feel free to put code styling and autoformatters on the PTG agenda | 12:24 |
opendevreview | Merged openstack/nova master: db: Replace use of backref https://review.opendev.org/c/openstack/nova/+/860829 | 12:25 |
bauzas | not sure my opinion will evolve but I'm still open to discuss about it | 12:25 |
bauzas | I just don't think the benefits overcome the pain | 12:25 |
sean-k-mooney | honestly the pain of not having them makes me somethimes condier if i wnat to keep manintianing nova | 12:26 |
bauzas | and about exceprts of code changes that pulled extra complexity into backports, I can do some homework and find some series | 12:26 |
sean-k-mooney | bauzas: i would happly backport the autofromating if that woudl make you feel beeter | 12:26 |
bauzas | not out of my head at first sight unfortunately, but there are experiences for sure | 12:26 |
bauzas | sean-k-mooney: down to Train ? :) | 12:26 |
sean-k-mooney | sure | 12:26 |
sean-k-mooney | the patches would be generated by the tool | 12:27 |
sean-k-mooney | we woudl jsut have to review it once and its done for ever | 12:27 |
sean-k-mooney | if that is your main objection its sovleabel. but im also oke with fixing this when backporting | 12:27 |
bauzas | that's indeed my main objection, beyond just a feeling | 12:28 |
sean-k-mooney | as you know i have been working in both go and python lately | 12:28 |
bauzas | I'm personnally against such things because I think it negates the stance of writing, but this is a personal opinion and absolutely not arguable | 12:29 |
sean-k-mooney | and ansible... but it has renifoced that feelting that auto liniting is good | 12:29 |
sean-k-mooney | and auto formating is both good for the reviewer and author | 12:29 |
bauzas | I can't wait for a ChatGPT bot on Visual Code | 12:30 |
bauzas | VS Code* | 12:30 |
stephenfin | https://marketplace.visualstudio.com/items?itemName=GitHub.copilotvs | 12:30 |
sean-k-mooney | hehe i already use code poilte ot help writhe my comments | 12:30 |
* bauzas facepalms | 12:31 | |
sean-k-mooney | its why they have had less typos lately | 12:31 |
bauzas | and yeah, I heard about GH CoPilot | 12:31 |
* bauzas is just about to give up when he reads this | 12:31 | |
sean-k-mooney | it daydreams a bit so unless you know the code base i would not recomend it to new commers | 12:32 |
sean-k-mooney | i have mostly been using it of go/ansible but i find i have to check its work and rewite it but its often close to what i would write | 12:33 |
bauzas | that's the problem with those AI tools | 12:33 |
bauzas | 95% of the job is perfectly done | 12:33 |
bauzas | and then the rest is errorprone | 12:33 |
sean-k-mooney | yep and that last 5% is where being a software engineer comes in | 12:33 |
bauzas | I'm not sure my productivity will raise because of a bot writing my code | 12:34 |
sean-k-mooney | using your expirnce ot understand its probmpts and correct them | 12:34 |
bauzas | most of the time it takes for me to write a feature is : 1/ identify the gaps, 2/ come with a proposal, 3/ let the community agree on the proposal, 4/ write that | 12:35 |
bauzas | the 4/ isn't exactly hard to do | 12:35 |
sean-k-mooney | sure and most of the process of adding a feature into nova is code review | 12:36 |
sean-k-mooney | not wirting the code in the first place | 12:36 |
bauzas | honestly, I feel the entry barrier on Nova isn't because we're not having black style and autoformatters | 12:37 |
bauzas | but rather because it's a very large distributed software contributed by many in different times | 12:38 |
bauzas | and I don't think autoformatters will lower this barrier | 12:38 |
sean-k-mooney | sure but that misses the point | 12:38 |
bauzas | which is that people can autotab their brackets ? | 12:39 |
sean-k-mooney | no | 12:40 |
stephenfin | it's a quality of life improvement | 12:40 |
sean-k-mooney | yep | 12:40 |
stephenfin | being a surgeon is hard: working in a clean operating theatre makes it slightly easier | 12:40 |
bauzas | I see myself more like a butcher :D | 12:41 |
bauzas | anyway, I stink and I need to get a shower | 12:41 |
stephenfin | then black is a sharp knife | 12:41 |
stephenfin | ;) | 12:41 |
stephenfin | or rather an automatic knife sharpener | 12:41 |
sean-k-mooney | i was going to play the frech card and say its a Charcuterie | 12:41 |
bauzas | stephenfin: if we continue the analogy, people can get hurt by a sharp knife if they don't know how to use it :) | 12:42 |
sean-k-mooney | which is another reason my analogy is better :) | 12:42 |
* bauzas disappears for 10 mins | 12:42 | |
sean-k-mooney | be a charcutier not a butcher of our lovely codebase :) | 12:43 |
sean-k-mooney | stephenfin: since your about can you look at https://review.opendev.org/c/openstack/nova/+/895655 | 12:55 |
sean-k-mooney | once thats merged i can propsoe the revert adn ping it to rodolfo and co so we can renable it once neutron says the test is fixed for ovn | 12:56 |
stephenfin | ack, done | 12:56 |
sean-k-mooney | that test is one of the thing that cased the hackign patch to fail | 12:56 |
sean-k-mooney | the other failure was volume detach related | 12:57 |
bauzas | fwiw, starting to look at the rest of sql14 series | 13:05 |
opendevreview | Felix Huettner proposed openstack/nova master: ignore errors when deleting volume attachments https://review.opendev.org/c/openstack/nova/+/896235 | 13:09 |
bauzas | okay, this is Friday, I'm not a better and decent position to start a debate https://luminousmen.com/post/my-unpopular-opinion-about-black-code-formatter | 13:18 |
bauzas | now* | 13:18 |
sean-k-mooney | for what its worht a number of opentack proejct have arelady adopted black | 13:18 |
bauzas | that doesn't make the point valid :) | 13:19 |
sean-k-mooney | we already have autopep8 which was a compomise | 13:19 |
bauzas | yup, and which doesn't force your code to change, right? | 13:19 |
bauzas | just because a ending bracket has to be on a separate line | 13:20 |
sean-k-mooney | it only fixes code if it not pep8 complient | 13:20 |
sean-k-mooney | and it undersands the things we disable in tox.ini | 13:20 |
bauzas | black is *very* noisy in terms of code change since it can't adapt to the existing code style | 13:20 |
sean-k-mooney | i.e. it reads https://github.com/openstack/nova/blob/master/tox.ini#L307 | 13:21 |
bauzas | and it forces us to do the way blacks thinks being the best | 13:21 |
sean-k-mooney | i think that is better then use disagreeing on what is the best style | 13:22 |
bauzas | btw. this twitter post which is replied by Guido is absolutly gold | 13:22 |
sean-k-mooney | bauzas: by the way i actully dont really care what the tool is provide we have one that does it | 13:23 |
bauzas | it shows the mindset of many developers : in order to follow *best* practices of coding, it says "install X and Y" | 13:23 |
bauzas | my head explodes when I read this | 13:23 |
bauzas | I thought for 2 decades that coding best practices were about code patterns and reusability | 13:23 |
sean-k-mooney | partly but we are not talking about best praticies | 13:24 |
sean-k-mooney | if you think we are then again your are missing the point | 13:24 |
bauzas | sean-k-mooney: read my link above, you'll see this tweet :) | 13:24 |
sean-k-mooney | i have seen those before | 13:24 |
bauzas | ouch https://black.readthedocs.io/en/stable/guides/introducing_black_to_your_project.html#avoiding-ruining-git-blame | 13:28 |
bauzas | sean-k-mooney: just in order to move on black and because I want to exactly understand the implications, do you know how the other projects manage the git blame mangling thing ? | 13:47 |
sean-k-mooney | i belvie you can annotate the commit to be ignored for blame | 13:48 |
sean-k-mooney | and it will then not be seein in github or locally by default | 13:48 |
sean-k-mooney | bauzas: which stephen did there https://review.opendev.org/c/openstack/nova/+/896231 | 13:49 |
bauzas | yeah, that's what black is saying on their docs | 13:49 |
bauzas | okay, then that's planned, gtk | 13:49 |
sean-k-mooney | yep its definetly a nice ux improvment ot have that | 13:56 |
sean-k-mooney | althoguh i woudl also take blame working in github at all on the libvirt driver... | 13:57 |
*** EugenMayer4401 is now known as EugenMayer440 | 16:04 | |
opendevreview | Merged openstack/nova master: Fix pep8 errors with new hacking https://review.opendev.org/c/openstack/nova/+/874517 | 17:34 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!