Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-06-27
14:51:08 edleafe I've stated my concerns
14:51:20 jaypipes I'll go back to trying to clean up the proposed code and add commentary to it.
14:51:30 dansmith jaypipes: before you do, we need to have a serious talk
14:51:37 edleafe if you still want to go down that route, I'll accept that
14:51:53 jaypipes dansmith: yes?
14:52:09 dansmith jaypipes: a couple months ago you started doing this trailing paren-on-a-line-by-itself thing, which I argued about, but let you continue for some large function calls in tests
14:52:22 dansmith jaypipes: but this patch has them all over the place and for calls with only a few parameters that could be a couple lines
14:52:35 dansmith jaypipes: _none_ of our other code looks like that and it's an abrupt change recently
14:52:50 dansmith it looks very different to everything else and it really (really) bugs me
14:53:28 jaypipes dansmith: k, I'll restyle
14:53:43 dansmith jaypipes: I'd *really* appreciate it :)
14:54:58 jaypipes dansmith: just for the parens, right? not the dict notation?
14:55:21 dansmith jaypipes: the parens are what make my eyes bleed, but give me a dict notation example
14:55:45 dansmith if you mean just ending on a line with } that's fine
14:55:54 jaypipes yeah
14:56:10 jaypipes k, will change.
14:56:17 dansmith yeah, we do that everywhere and that's cool
14:56:39 jaypipes dansmith: no prob, will fix.
14:56:45 jaypipes dansmith: just for the record, I find this:
14:56:47 jaypipes resource_query = ",".join(
14:56:47 jaypipes sorted("%s:%s" % (rc, amount)
14:56:47 jaypipes for (rc, amount) in resources.items())
14:56:47 jaypipes )
14:56:55 dansmith le barf
14:56:59 jaypipes easier to read than this:
14:56:59 jaypipes resource_query = ",".join(sorted("%s:%s" % (rc, amount)
14:57:00 jaypipes for (rc, amount) in resources.items()))
14:57:10 dansmith you can make it three lines,
14:57:19 dansmith just don't put the trailing paren on a line by itself
14:57:30 jaypipes loud and clear sir.
14:57:34 dansmith but even that is slightly less smelly than:
14:57:37 dansmith myfunc(
14:57:40 dansmith foo, bar,
14:57:41 dansmith )
14:57:45 dansmith le double barf ^
14:57:49 jaypipes heh
15:03:56 openstackgerrit Andreas Scheuring proposed openstack/nova master: DO NOT MERGE! Dummy patch https://review.openstack.org/478186
15:08:31 efried dansmith +1, thanks for bringing a hammer to that party.
15:09:09 dansmith efried: interventions are sometimes necessary to correct misbehavior
15:09:48 dansmith I learned that from TLC
15:09:54 efried The more you know
15:10:15 dansmith the L used to stand for learning, but now it stands for something else like "look-at-the-freaks"
15:11:34 jaypipes dansmith: https://review.openstack.org/#/c/475448/13/nova/tests/functional/api/openstack/placement/fixtures.py
15:11:35 efried Never had the cable, so I'll have to take your word for it. (I have absolutely no trouble wasting enough time with Netflix and Amazon Prime.)
15:11:56 jaypipes dansmith: do you want me to de-parens those fixutres like on line 262?
15:13:09 dansmith jaypipes: yes, gross
15:13:30 jaypipes dansmith: ok.
15:13:55 jaypipes dansmith: so it's really just the lone paren on a line that you don't like, right?
15:14:25 dansmith jaypipes: and the trailing comma in a call that goes with it
15:14:37 efried I'll restate dislike for the trailing comma, which loses its benefit (two-line change becomes one-line change) if you pull the paren back.
15:14:40 efried yeah, what he said.
15:15:07 dansmith jaypipes: I don't really see the need to break every call into a multiline one-per-line verbose list either, but.. that's less smelly than the lone paren and trailing comma thing
15:15:14 dansmith for long ones, or ones with a very long parameter name, sure
15:15:30 jaypipes different style, is all.
15:15:37 jaypipes but I'll work with you.
15:15:49 dansmith jaypipes: I know, and we do some of that in other code.. we do not, however, do the trailing comma and lone paren thing :)
15:16:03 openstackgerrit Eric Fried proposed openstack/nova master: Add formatting to scheduling activity diagram https://review.openstack.org/476204
15:16:55 jaypipes dansmith, efried: you guys would hate Golang then :)
15:16:57 efried I personally dig the one-per-line thing for really long param lists. I'll take the vspace hit for readability there.
15:17:18 dansmith jaypipes: it's consistency with the rest of our code that I care about
15:17:24 jaypipes understood.
15:17:29 dansmith jaypipes: if all our code looked like that it would be fine (ugly, but fine)
15:17:50 dansmith efried: for long param lists, I do it myself sometimes, yeah
15:25:35 jaypipes dansmith: I'\
15:25:35 jaypipes m goin\
15:25:35 jaypipes g to start codi\
15:25:35 jaypipes ng like this just fo\
15:25:35 jaypipes r you.
15:25:53 dansmith jaypipes: I don't hate that, but it's not our style, so I'll still -1 it :)
15:25:58 jaypipes lol
15:26:09 dansmith er wait, you probably didn't get that, let me try again:
15:26:11 dansmith jaypipes: I don't hate that, but it's not our style, so I'll still -1 it :
15:26:12 dansmith )
15:26:13 dansmith there
15:26:21 dansmith FTFY
15:26:54 jaypipes rofl
15:27:39 jaypipes dansmith: you missed the comma at the end of the first line.
15:28:49 jaypipes dansmith: you're my little exorcist today.
15:29:15 dansmith jaypipes: dude, https://review.openstack.org/#/c/476630/6/nova/tests/unit/scheduler/weights/test_weights_affinity.py on line 50 and below
15:29:22 dansmith why do you hate parens all the sudden?
15:29:30 dansmith stop torturing parens, jaypipes
15:30:35 jaypipes dansmith: alignment is very important to me.
15:31:26 openstackgerrit Merged openstack/python-novaclient master: Updated from global requirements https://review.openstack.org/478083
15:31:31 dansmith jaypipes: you can align those without putting parens on their own lines
15:31:37 jaypipes dansmith: but I'm gonna rework that patch actually to reduce the amount of code change as much as possible, even if it means some hackery.
15:31:41 dansmith you've actually increased indent there for no reason :)
15:32:22 dansmith jaypipes: okay, it is a lot of change, but I'm not super opposed to the way it is, FWIW
15:32:57 jaypipes dansmith: I see your suggestion about "just return the HostState objects". That's more along the lines of what I was thinking.
15:33:13 dansmith I'm glad you let the golang thing slip, as that completes the circle of me wondering what in your life happened in the last couple months that caused you to start doing this
15:33:21 dansmith jaypipes: yeah
15:33:22 jaypipes dansmith: :)
15:40:29 openstackgerrit Ildiko Vancsa proposed openstack/nova master: Add Cinder v3 detach to swap_volume https://review.openstack.org/456971
16:26:11 evilpig_ hello, does anyone know if the xen backend is supported for vcpu pinning?
16:26:41 evilpig_ i think i have everything set up right, but the numatopologyfilter is rejecting my compute nodes
16:27:23 evilpig_ i can perform the pinning through virsh so it looks like libvirt support is there
16:28:00 openstackgerrit Merged openstack/nova master: Updated from global requirements https://review.openstack.org/477982
16:28:29 artom evilpig_, I don't think so
16:28:35 artom evilpig_, https://docs.openstack.org/developer/nova/feature_classification.html#matrix-nfv
16:28:46 artom That doesn't answer your exact question
16:29:32 evilpig_ artom, thank you for the pointer

Earlier   Later