| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-31 | |||
| 12:22:50 | sean-k-mooney | hehe mostly true | |
| 12:22:54 | sean-k-mooney | self.task.compute_rpcapi.finish_revert_snapshot_based_resize_at_source.assert_called_once_with( | |
| 12:23:01 | sean-k-mooney | that is an example of something it cant fix | |
| 12:24:44 | opendevreview | ribaudr proposed openstack/python-novaclient master: Microversion 2.91: Support specifying destination host to unshelve https://review.opendev.org/c/openstack/python-novaclient/+/831651 | |
| 12:24:51 | sean-k-mooney | i am manually fixign those like this | |
| 12:24:54 | sean-k-mooney | mock_finish_at_source = ( | |
| 12:24:56 | sean-k-mooney | self.task.compute_rpcapi.finish_revert_snapshot_based_resize_at_source | |
| 12:24:58 | sean-k-mooney | ) | |
| 12:25:00 | sean-k-mooney | mock_finish_at_source.assert_called_once_with( | |
| 12:40:02 | opendevreview | ribaudr proposed openstack/nova master: Allow unshelve to a specific host https://review.opendev.org/c/openstack/nova/+/831507 | |
| 12:47:49 | opendevreview | Balazs Gibizer proposed openstack/osc-placement master: Support microversion 1.39 https://review.opendev.org/c/openstack/osc-placement/+/828545 | |
| 12:48:05 | opendevreview | Balazs Gibizer proposed openstack/osc-placement master: Support microversion 1.39 https://review.opendev.org/c/openstack/osc-placement/+/828545 | |
| 12:50:20 | Uggla | sean-k-mooney, pep8 from the end of file, good idea. | |
| 12:50:53 | opendevreview | Balazs Gibizer proposed openstack/nova stable/xena: Add service version check workaround for FFU https://review.opendev.org/c/openstack/nova/+/831174 | |
| 12:51:40 | Uggla | gibi, a black patch ? To get rid of formatting ? | |
| 12:51:50 | gibi | Uggla: yepp | |
| 12:52:08 | sean-k-mooney | well more make a tool do it | |
| 12:52:26 | Uggla | gibi, oh, that will be really cool. | |
| 12:54:50 | gibi | sean-k-mooney: even the best hero needs help from a sidekick \ | |
| 13:00:48 | sean-k-mooney | one thing i dont like but i dont see a way to avoid is i have to #noqa: E501 lambdas | |
| 13:01:11 | sean-k-mooney | i.e. if a lamda defition does not fit on one line i have to ignore the line lenght | |
| 13:01:31 | sean-k-mooney | since there is not a valid way to break it up that black wont undo | |
| 13:02:07 | sean-k-mooney | there might be a config setting i could tweak in a followup but for now im just ignoring the lenght check in that specific case | |
| 13:03:17 | sean-k-mooney | the other option is to allow lamdas to be named but we block that currently | |
| 13:03:30 | sean-k-mooney | so i cant just pull it out into a varible or similar | |
| 13:03:52 | sean-k-mooney | and pulling them out into private fucntiosn is more then i want to do in this patch | |
| 13:04:16 | sean-k-mooney | im really trying to keep the manual changes as small as possible | |
| 13:04:45 | sean-k-mooney | we can always go back and fix that later by looking for the #noqa comments | |
| 13:07:59 | gibi | yeah, I think the official suggestion is that if it does not fit into a line then it should be def | |
| 13:08:13 | gibi | but I agree not do change everything in one go | |
| 13:44:16 | bauzas | sean-k-mooney: gibi: fwiw, I loudly replied to the E501 thread | |
| 13:45:26 | bauzas | tl:dr: this is a at least a bikeshed and maybe a pandora box that will generate a lot of efforts for something needless | |
| 13:45:37 | opendevreview | Merged openstack/nova stable/ussuri: [CI] Install dependencies for docs target https://review.opendev.org/c/openstack/nova/+/839813 | |
| 13:48:53 | sean-k-mooney | bauzas: perhaps but im still going to finish my nova black patch | |
| 13:49:25 | sean-k-mooney | bauzas: dealing with formating it perhaps the thing i hate most about working on nova other then the dowstream paperwork | |
| 13:50:01 | bauzas | sean-k-mooney: take it as you wish, but I won't accept any change touching this limit | |
| 13:50:31 | bauzas | this will also break style consistency between projects and some people using terms with 80 chars will see a difference | |
| 13:51:07 | bauzas | while the other way is just because people feel useless to have 80 chars on a 1440 display | |
| 13:51:28 | bauzas | this is maybe useless, but changing the default has a cost I'm not ready to pay | |
| 13:52:17 | bauzas | and I wish our contributor energy would be better used to some other things but just a stylish nitty modification | |
| 13:52:45 | bauzas | pro-tip : 80-char limit allows you to open multiple files in parallel, that being said | |
| 13:54:16 | bauzas | btw. sean-k-mooney you wanted me to review some os-vif changes | |
| 13:54:28 | bauzas | that looks to me better use of my worktime | |
| 13:56:44 | sean-k-mooney | bauzas: i can reduce it down to 79 then and keep the limit | |
| 13:57:03 | sean-k-mooney | the black defaul is 88 | |
| 13:57:23 | sean-k-mooney | which works fine even on 1080 screesn or my phone | |
| 13:57:38 | sean-k-mooney | which is actully 1080 on landscpae so i guess thats the same | |
| 13:58:30 | bauzas | sean-k-mooney: changing our pep8 tool is a totally different effort than changing the limit | |
| 13:58:36 | sean-k-mooney | unless you have a 720p screen you can open two 90 char termins in a 1080 screen and still have space | |
| 13:58:51 | sean-k-mooney | bauzas: its not changing the pep8 tool by the way | |
| 13:58:59 | sean-k-mooney | its still using flake8 | |
| 13:59:00 | bauzas | sean-k-mooney: sorry I meant the formatter | |
| 13:59:22 | sean-k-mooney | but isnt of autopep8 which does minimal formating it would be black which does everything for you | |
| 13:59:58 | bauzas | sean-k-mooney: if black stays optional, I'm not opposed to it | |
| 14:00:12 | bauzas | sean-k-mooney: are you planning to make it a tox target ? | |
| 14:00:17 | sean-k-mooney | in terms of developer mental healt not having auto formating is defferntly bad for burn out | |
| 14:00:41 | sean-k-mooney | bauzas: it would have to be enforced in ci | |
| 14:00:47 | bauzas | well, I certainly had burnout conditions in the past that didn't occur because of the formatting... | |
| 14:00:48 | sean-k-mooney | otherwise there is no real point | |
| 14:01:21 | sean-k-mooney | i condiered stopping working on nova because of the effort required to get patches merged much of which was becasue of style considerations | |
| 14:01:45 | bauzas | a formatter wouldn't help, no ? | |
| 14:02:00 | sean-k-mooney | it would | |
| 14:02:13 | sean-k-mooney | auto fromating would remove debates of how to format thigns | |
| 14:02:19 | sean-k-mooney | its what the tool does and done | |
| 14:02:44 | bauzas | I'm certainly not one who debated the stylish things | |
| 14:03:16 | bauzas | for line limits, I'm not opposed to use either brackets or backslashes, eg. | |
| 14:03:24 | bauzas | or even local variables | |
| 14:03:41 | bauzas | at least, I could comment on it, but I'm not *opposed* to it | |
| 14:04:28 | bauzas | so, if you felt exhausted by those debates, I really feel your frustration and I think we should rather document the fact that we shouldn't enforce exact styling guidelines | |
| 14:05:00 | bauzas | yet again a code review issue and not a tool miss | |
| 14:05:10 | bauzas | (not saying pypià | |
| 14:05:10 | bauzas | (not saying pypià | |
| 14:05:13 | bauzas | ;) | |
| 14:20:22 | dansmith | sean-k-mooney: bauzas: the other day we were talking about something related to file-backed memory support in libvirt/nova | |
| 14:20:47 | dansmith | I don't remember for what, but I wrote something to get support started for the guy that added that support, which never got merged: https://review.opendev.org/c/openstack/devstack/+/574792 | |
| 14:21:00 | kashyap | sean-k-mooney: What is the "black default is 88"? | |
| 14:21:10 | dansmith | it just got some "is this still interesting" action recently, so... is it worth getting that merged? | |
| 14:21:19 | bauzas | kashyap: that means by default black generates lines of 88 chars | |
| 14:21:37 | bauzas | dansmith: reloading the context | |
| 14:21:39 | kashyap | I don't know what is "black" here. Maybe I'm being too dense | |
| 14:21:47 | bauzas | kashyap: https://pypi.org/project/black/ | |
| 14:22:02 | kashyap | Ah, it's a tool! | |
| 14:22:06 | kashyap | Thanks | |
| 14:23:34 | bauzas | kashyap: and there is blue, a fork of black https://pypi.org/project/blue/ | |
| 14:23:57 | bauzas | see, we diverted from a very interesting ping from dansmith | |
| 14:24:01 | kashyap | bauzas: Yeah, I recall seeing that in passing; th | |
| 14:24:01 | dansmith | black's coding style is so ugly | |
| 14:24:05 | kashyap | s/th/thx/ | |
| 14:24:09 | bauzas | dansmith: stay, please | |
| 14:24:26 | bauzas | I need to convince a few people here | |
| 14:25:29 | bauzas | dansmith: do you remember if we discuss the file-backed memory case in a meeting or somewhere else ? | |
| 14:25:51 | dansmith | bauzas: it was downstream in a meeting I think, I don't remember the context | |
| 14:25:57 | bauzas | hah | |
| 14:26:23 | bauzas | well, I'd say this is just a libvirt knob | |
| 14:26:34 | bauzas | so, to answer your question, worth merging yeah | |
| 14:26:39 | dansmith | aight | |
| 14:27:09 | dansmith | presumably we need a test case that uses/enables it | |
| 14:27:14 | bauzas | I like the 'you can't overcommit memory if you file-back your memory" ting | |
| 14:27:16 | dansmith | which probably existed as a depends-on to this somewhere | |
| 14:29:08 | bauzas | yeah, this is a very old patch | |