| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-25 | |||
| 14:12:57 | stephenfin | jangutter: Some small comments in that patch ^ | |
| 14:15:56 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Fix nits in brctl removal (vif_plug_linux_bridge) https://review.openstack.org/639099 | |
| 14:16:13 | jangutter | stephenfin: ask and ye shall receive. | |
| 14:16:20 | sean-k-mooney | jangutter: damb looks like i missed leaving the comment by a few seconds | |
| 14:16:54 | sean-k-mooney | jangutter: can ou add the comment too? thanks for doing this by the way | |
| 14:17:23 | jangutter | sean-k-mooney: hahaha, no worries. I don't have my Nagle algorithm turned on today. | |
| 14:17:34 | stephenfin | jangutter: is that correct? | |
| 14:17:48 | stephenfin | x, = (1, 2) gives a ValueError | |
| 14:17:59 | stephenfin | You'd need 'x, _' | |
| 14:18:20 | jangutter | stephenfin: frack, you're right. | |
| 14:18:44 | jangutter | stephenfin: just goes to show that code path is untested so badly. | |
| 14:18:51 | stephenfin | Oh, found an ancient change of mine. does this docs patch make sense to you, jaypipes? https://review.openstack.org/#/c/445436/ | |
| 14:19:32 | sean-k-mooney | oh thats to nova i was like i dont remember that | |
| 14:23:14 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Fix nits in brctl removal (vif_plug_linux_bridge) https://review.openstack.org/639099 | |
| 14:23:51 | jangutter | stephenfin, sean-k-mooney: 3rd time lucky Monday? | |
| 14:27:03 | jangutter | I think that code is dead since before I touched OpenStack, but I'm worried that breaking it will cause CERN to accidentally create a black hole or something. | |
| 14:30:23 | openstackgerrit | Ghanshyam Mann proposed openstack/nova stable/rocky: DNM: Testing nova legacy jobs on rocky still use xenial https://review.openstack.org/639107 | |
| 14:31:57 | sean-k-mooney | jangutter: the linux bridge pluging is partcalarly archane as we spend much less time maintaining it. im hoping that by sharing code with the ovs plugin in train we can impove the quality of both | |
| 14:32:38 | sean-k-mooney | jangutter: i would also like to add an sriov plugin intree in train so we can finally remove the last of the vif.py legacy code from nova | |
| 14:32:55 | jangutter | sean-k-mooney: ++ on both of those. | |
| 14:33:43 | sean-k-mooney | we have avoided sharing code so people can copy a sub directly and use that as a base for a new plugin but instead i would like to create a cookiecutter template for that and document it | |
| 14:34:22 | sean-k-mooney | *sub directory | |
| 14:38:11 | zhubx007 | mdbooth: as you say, it is ok when I set force_raw_images to True. Whether it is great to add checking the image format when call import_image under rbd imagebackend because only raw is good for this backend. | |
| 14:39:23 | openstackgerrit | Yongli He proposed openstack/nova master: Add server sub-resource topology API https://review.openstack.org/621476 | |
| 14:40:05 | openstackgerrit | sean mooney proposed openstack/os-vif master: make functional tests run on python 3 https://review.openstack.org/638053 | |
| 14:40:06 | openstackgerrit | sean mooney proposed openstack/os-vif master: modify functional base.py to allow using vscode https://review.openstack.org/638058 | |
| 15:04:20 | mriedem | jaypipes: efried: i'm +2 on the current bottom 4 bw provider series changes https://review.openstack.org/#/c/616240/ - nits to be sure but can be addressed in another follow up i think | |
| 15:04:41 | mriedem | i think the api change is the goal for stein https://review.openstack.org/#/c/636360/ so i'm trying to do a review push to get there this week | |
| 15:10:11 | openstackgerrit | Kashyap Chamarthy proposed openstack/nova master: libvirt: Rework 'EBUSY' (SIGKILL) error handling code path https://review.openstack.org/639091 | |
| 15:10:53 | kashyap | mriedem: Blast from the past ^ | |
| 15:11:13 | efried | mriedem: ack. Trying to figure out if it's mathematically possible to have more fups than patches. | |
| 15:11:29 | mriedem | efried: i made a similar joke on one of these | |
| 15:11:32 | mriedem | 1:1 fups | |
| 15:11:52 | efried | I hate being unoriginal. Now I will have to kill you. | |
| 15:12:09 | efried | Never mind, I'll need you to review my code. You live. | |
| 15:16:15 | gibi | efried, mriedem: I working on a fup that will contain fixes up til https://review.openstack.org/#/c/573317 | |
| 15:16:26 | kashyap | What is a "fup"? | |
| 15:16:33 | gibi | kashyap: Follow up | |
| 15:16:39 | kashyap | My tool doesn't know it: | |
| 15:16:39 | kashyap | $> wtf is fup | |
| 15:16:40 | kashyap | wtf: I don't know what `fup' means! | |
| 15:16:45 | kashyap | gibi: Ah, thanks. | |
| 15:16:50 | efried | or what you were thinking, depending. | |
| 15:16:54 | sean-k-mooney | fix up patch or follow up patch | |
| 15:17:02 | sean-k-mooney | kashyap: ^ | |
| 15:17:21 | kashyap | Please, "follow-up" as keyword. | |
| 15:17:37 | sean-k-mooney | no one uses that^ | |
| 15:17:39 | kashyap | As I accidentally looked at the definition of "fup" on Urban Dictionary ... and "you won't believe what happens next!" | |
| 15:17:49 | kashyap | sean-k-mooney: Yeah, yeah. I know it's just IRC slang | |
| 15:18:45 | efried | Back in the day it was used by IBM support teams for APARs, in caps (FUP), simply shorthand for "follow up", as in "FUP with customer..." | |
| 15:18:45 | efried | and led to all kinds of raunchy (well, for IBM, anyway) jokes. | |
| 15:18:51 | sean-k-mooney | yep and recent slang at that. but i dont think we need to add follow-up: someithg to the commits | |
| 15:19:17 | kashyap | I see | |
| 15:19:39 | efried | Agree with that; though I have to say I'm not a fan of commit titles like "Follow on for <change-id>" with nothing in the body of the message. | |
| 15:20:10 | efried | When I do these, I tend to do "FUP for <brief description>" and then enumerate links to specific comments I'm responding to in the body. | |
| 15:20:16 | efried | and of course, everybody should do it my way. | |
| 15:20:57 | sean-k-mooney | haha well that at least makes sense | |
| 15:21:23 | efried | example: https://review.openstack.org/#/c/600474/ | |
| 15:22:44 | sean-k-mooney | i think this is fine too https://review.openstack.org/#/c/639099/ but a midpoint betwen the too is praobly good too. | |
| 15:23:14 | kashyap | efried: Yeah, I fully agree on "bad commit messages". Reminds me of this excellent guide written by DanPB many moons ago: https://wiki.openstack.org/wiki/GitCommitMessages | |
| 15:23:23 | kashyap | People should take that guide to the heart. | |
| 15:23:24 | sean-k-mooney | the disadvantage of both are you have to click the link/change id to see the context | |
| 15:24:02 | sean-k-mooney | kashyap: there are some thing in the bad examples that i dont fully agree with in that guide but i do reference it frequently | |
| 15:24:03 | kashyap | (And of course this excellent post from a certain Chris Beans: https://chris.beams.io/posts/git-commit/) | |
| 15:24:41 | kashyap | sean-k-mooney: The fundamental rule I follow in commit messages is: provide all the context right there in the commit message. | |
| 15:24:45 | efried | kashyap: I frequently reference the GitCommitMessages wiki page. (That link showed up purple for me :) | |
| 15:24:47 | kashyap | If you have to link to something, summarize it | |
| 15:25:20 | kashyap | (E.g. faciliate the poor suckers working without internet connection and are doing `git log` sleuthing) | |
| 15:25:34 | kashyap | efried: Cool :-) | |
| 15:25:53 | kashyap | sean-k-mooney: That point is actually point 2 in that Wiki page: "Do not assume the reviewer has access to external web services/site." | |
| 15:25:56 | sean-k-mooney | kashyap: one thing that i always tought that wiki discuoraged was bullet point list in the commit which i think are actully good style | |
| 15:26:14 | sean-k-mooney | yep | |
| 15:26:37 | kashyap | sean-k-mooney: Bullet points are fine -- as long as they are complete, and _coherent_ sentences | |
| 15:26:44 | kashyap | And not no-assed "thoughts" | |
| 15:26:48 | sean-k-mooney | so i think https://review.openstack.org/#/c/638058/2//COMMIT_MSG is a good commit message but that general styple would be discuraged | |
| 15:27:29 | kashyap | sean-k-mooney: Was actually reading that change | |
| 15:28:14 | efried | My understanding is that what's discouraged is changing more than one thing in a commit. A bullet list sometimes (but definitely not always) indicates that that's happening. But a bullet list isn't inherently bad. | |
| 15:28:41 | efried | be like saying, "never use the word 'also' in a commit message" | |
| 15:29:00 | sean-k-mooney | efried: ya that is true the distinciton is not made clear in the example in the wiki | |
| 15:29:31 | kashyap | sean-k-mooney: I have a small 'issue' with that vscode commit message | |
| 15:30:08 | kashyap | It doesn't follow the general "scheme" most of the human brains are used to: describe the problem, then tell the solution. | |
| 15:30:10 | sean-k-mooney | kashyap: there are 2 thing i need to fix | |
| 15:30:26 | sean-k-mooney | kashyap: so leave a comment and ill adress them shortly | |
| 15:30:26 | kashyap | Lastly, this is my Git commit message 'scheme': | |
| 15:30:26 | kashyap | [One line summary -- in imperative mood] | |
| 15:30:26 | kashyap | Describe the problem. | |
| 15:30:28 | kashyap | Describe your solution. And more importantly, tell *why*. | |
| 15:31:18 | kashyap | (You do that, though. But all of them in bullets :-)) Anyway, don't want to belabor on this. | |
| 15:32:08 | stephenfin | bauzas: Fancy doing me the honour? https://review.openstack.org/#/c/445436/ | |
| 15:32:59 | bauzas | stephenfin: with pleasure | |
| 15:33:43 | bauzas | stephenfin: oh wait, sec. the option is *already* deprecated ? | |
| 15:33:52 | bauzas | stephenfin: if so, it should have a reno note | |
| 15:33:56 | bauzas | if not, we need it | |
| 15:34:03 | stephenfin | that's a good point | |
| 15:34:09 | stephenfin | lemme fix that | |
| 15:35:36 | bauzas | stephenfin: will update the commit msg to stop the zuul check then | |
| 15:35:52 | stephenfin | If you remove the -W, it'll take it out of the gate | |
| 15:35:56 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: conf: Improve documentation for defer_iptables_apply https://review.openstack.org/445436 | |
| 15:37:35 | bauzas | stephenfin: nope, unless zuul changed | |
| 15:37:58 | bauzas | for pushing the change out of the gate pipeline, we need to have a new revision | |