Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-05-31
09:52:02 gibi I just wanted to make an agreement first
09:52:16 sean-k-mooney ok i think that will be better longterm
09:52:26 gibi yepp
09:52:48 sean-k-mooney by the way on a related not stephenfin made an intersting point on the mailing list
09:53:08 sean-k-mooney apprently we can now tell git blame to ignore commits
09:53:31 sean-k-mooney so if i was to push a ptach to pre-commit normalis all the quotes for example
09:53:35 gibi yepp I saw. I still think it will be a big debate to blackify nova
09:53:39 sean-k-mooney we can tell git blame to ignore that
09:53:51 sean-k-mooney oh i was not plannign to add black
09:54:03 sean-k-mooney jus ta few more pre-comit hooks
09:54:32 sean-k-mooney using black is an option i guess but i wanteed to avoid the linelenth debate
09:54:44 gibi I'm a bit of all or nothing. as any normalization gives us the backport conflict so why not then do all the normalization at once
09:54:53 sean-k-mooney and just make some small imporvements
09:54:58 gibi black handles linelegth
09:55:07 gibi you can ask it to restrict to 79
09:55:08 sean-k-mooney i would like to do all
09:55:24 sean-k-mooney i really really wish we had auto code formating
09:55:33 gibi [default: 88]
09:55:33 gibi -l, --line-length INTEGER How many characters per line to allow.
09:55:45 gibi what an insteresting default :)
09:55:48 sean-k-mooney but i dont want to break people's workflow just for my comfort
09:56:04 sean-k-mooney gibi: there was an articl on why that was chosen i think
09:56:22 sean-k-mooney gibi: the issue is i dont think there is currently a way to set that in a file
09:56:23 gibi yeah now that you said it
09:57:18 sean-k-mooney oh the other hand i think have autoformating using any tool woudl improve new contibutor experince
09:57:23 sean-k-mooney and all our live in the long run
09:57:27 gibi I totally agree
09:57:44 gibi I just want a single commit to reformat everything with a tool and then forget about it
09:58:02 gibi gradual change will mean a lot of commit to ignore later
09:58:57 sean-k-mooney yep same
09:59:03 sean-k-mooney also https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length
09:59:20 sean-k-mooney apprently it now follow the flak8 max-line-lenth
09:59:25 sean-k-mooney which was missing for so long
09:59:53 gibi I still use https://github.com/gibizer/zuul-log-search/blob/main/tox.ini#L30
10:00:20 sean-k-mooney yep we coudl do that but the disadvantage is
10:00:24 sean-k-mooney it wont work for ides
10:00:28 sean-k-mooney like vscode
10:00:58 sean-k-mooney im using nano/emacs so not an issue for me personally i would be runnign black with precommit
10:01:25 sean-k-mooney but ya go eat. i can wip up a patch if we want to consider it
10:01:38 sean-k-mooney im not really show how it would look
10:01:56 gibi ack
11:43:08 stephenfin gibi: sean-k-mooney: If you do normalization, I see no reason not to apply that normalization to stable branches also
11:43:29 sean-k-mooney hum we coudl i guess
11:43:37 gibi including downstream? :)
11:43:40 sean-k-mooney im just fixign a few long lines on a black patch
11:43:54 sean-k-mooney gibi: it woudl get imported downstream if we did it upstream
11:43:59 sean-k-mooney but also yes
11:44:02 gibi sean-k-mooney: ahh good point
11:44:04 stephenfin gibi: yeah, what sean-k-mooney says
11:44:58 stephenfin I mean, in theory black's "slow" formatter checks to make sure the AST is identical before and after so there will be no functional differences
11:45:20 sean-k-mooney yep that is true for the most part
11:45:35 sean-k-mooney also its faster tehn autopep8
11:45:43 sean-k-mooney or flake8
11:45:45 gibi there will be a sh*tload of merge conflicts in open reviews but that is just one-time pain
11:46:19 gibi and easy to resolve by rebase, run black, push
11:54:06 sean-k-mooney the patch is pretty bix but the list of files where i actully need to make code chages by intoducing new varbailes is pretty small
11:54:08 sean-k-mooney https://paste.opendev.org/show/be8e3ZYGZeIjFv8uOcvz/
11:54:31 sean-k-mooney well that or () to put things onto a new line
11:55:00 sean-k-mooney black was able to handel almost all of it its slef but some of our mock names are kind of long
11:57:14 sean-k-mooney over all the code is looking quite readable after the format
11:57:51 sean-k-mooney stephenfin: you will like htat it seam to be quite close to your coding style already
11:59:08 stephenfin sean-k-mooney: yeah, I've been slowly forcing myself to move to that style of late since it seems to be getting pretty widespread now
11:59:25 sean-k-mooney same but mainly because of you
11:59:49 sean-k-mooney to premempt you asking me to change things in nits :)
12:00:05 gibi :)
12:00:06 sean-k-mooney got to make you work harder for those -1 stats :)
12:00:48 gibi sean-k-mooney: did you moved to 88 char line limit as the paster suggests?
12:01:00 sean-k-mooney ya
12:01:13 sean-k-mooney so the reason i did that is to mvoe to 79 with black
12:01:23 sean-k-mooney i would need to add project.toml
12:01:29 sean-k-mooney which i could do
12:01:36 sean-k-mooney but that seam like an even bigger change
12:01:51 gibi I would add -l 79 to the black command line as a stopgap
12:01:55 sean-k-mooney once i have this passing for 88 i can add a scond commit for 79
12:02:17 sean-k-mooney if we want 79 we could sqaush them
12:03:00 sean-k-mooney gibi: i have configured flak8 for 88 too in this patch
12:03:15 sean-k-mooney so im not removing flak8 or hacking
12:03:35 sean-k-mooney they will still enforce checks but im makign them compatibale with black
12:03:36 gibi ahh I see
12:03:59 gibi I'm OK with the two commit to review but I would like to squash before we merge
12:04:25 sean-k-mooney yep so the reason im going to do two commits is 1 we can decide if we prefer one vs the other
12:04:52 sean-k-mooney and two i know i will likely have to resolve more long nested calls for 79
12:05:15 sean-k-mooney i.e. obj.sub_obj.long_function_name....
12:06:16 sean-k-mooney our convention of mock_full_long_function_name.return_value.assert_called_once_with(
12:06:28 sean-k-mooney sometimes is over 88 when all on one line
12:06:35 gibi I see
12:09:32 sean-k-mooney lol i just opend the libvirt driver unit test and emacs warned me of performance issues with large files
12:10:27 sean-k-mooney to be fiar to ti it is 32k lines long
12:10:57 sean-k-mooney we really do need to decompsoe the libvirt driver into more moduels and split up the unit test file eventually
12:11:08 sean-k-mooney its just a lot of merge conflicts
12:12:23 sean-k-mooney its one of those things that we just need to decide to do at teh end or start of a cycle
12:14:32 gibi sean-k-mooney: I totally agree. pycharm simply dies on that file :D
12:21:28 sean-k-mooney i dont know if im the only one that does this but one trick i have adopted recently for fixing pep8 issue is starting form the end fo the file and working backwards
12:21:45 sean-k-mooney that way the earlier errors dont change line number
12:22:18 gibi good idea :)
12:22:34 gibi if we merge the black patch you can forget this trick :)
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

Earlier   Later