| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-25 | |||
| 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 | kashyap | Describe the problem. | |
| 15:30:26 | kashyap | [One line summary -- in imperative mood] | |
| 15:30:26 | kashyap | Lastly, this is my Git commit message 'scheme': | |
| 15:30:26 | sean-k-mooney | kashyap: so leave a comment and ill adress them shortly | |
| 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 | |
| 15:38:05 | bauzas | anyway, it's done | |
| 15:46:03 | openstackgerrit | Lajos Katona proposed openstack/python-novaclient master: Add support for microversion v2.70 https://review.openstack.org/637234 | |
| 15:54:43 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Deprecated 'defer_iptables_apply' https://review.openstack.org/445436 | |
| 15:54:46 | stephenfin | bauzas, jaypipes: That should be better ^ | |
| 15:55:27 | stephenfin | sean-k-mooney tells me what I'm doing is likely correct and I'm going to b̶l̶a̶m̶e̶ ̶h̶i̶m̶ ̶i̶f̶ ̶I̶'̶m̶ ̶w̶r̶o̶n̶g̶ trust him :) | |
| 15:56:18 | jaypipes | heh | |
| 15:59:23 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add unit tests for missing VirtualInterface in 2.70 os-interface https://review.openstack.org/639141 | |
| 16:00:02 | mriedem | can i get another core on the 'expose virtual device tags' api change? https://review.openstack.org/#/c/631948/ alex +2ed it and it's pretty straight-forward. last day in the runway queue for this one. | |
| 16:00:14 | stephenfin | sean-k-mooney: My docs are "nice to have". I feel cheated, Good Sir ;) | |
| 16:00:20 | stephenfin | mriedem: I can take a gawk | |
| 16:00:25 | mriedem | thanks | |
| 16:01:37 | yonglihe | this one in same situation: https://review.openstack.org/#/c/621474/22 | |
| 16:01:38 | sean-k-mooney | stephenfin: docs chagnes are non fuctional and can can be merged after the client lib freeze. since we generally just use the latest version of the docs people should still see them but they also can be backported to stable branches more easily so nice to have | |
| 16:02:08 | sean-k-mooney | stephenfin: hehe but i also think they will be straigt forward to merge so im going to go review them shortly anyway | |
| 16:02:16 | mriedem | yonglihe: i'll try to get to that one | |
| 16:03:23 | yonglihe | thanks. | |
| 16:04:26 | yonglihe | i suppose all stuff piled up at end of dev cycle, sorry for that. | |
| 16:12:37 | mriedem | np, it always happens | |
| 16:20:50 | bauzas | stephenfin: +Waboom | |
| 16:25:33 | openstackgerrit | Merged openstack/nova master: Refactor "networks" processing in ServersController.create https://review.openstack.org/633594 | |
| 16:27:31 | stephenfin | bauzas: Tank u | |
| 16:28:23 | stephenfin | (O_o_o_o_o_O) | |
| 16:28:23 | stephenfin | .-='=='==-, " | |
| 16:28:23 | stephenfin | .--._____, | |
| 16:33:29 | yonglihe | -:) | |
| 16:40:09 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Add 'flavor-extra-spec-image-property-validation' spec https://review.openstack.org/638734 | |
| 16:42:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: [Doc] Best practices for effectively tolerating down cells https://review.openstack.org/638173 | |
| 16:43:11 | tssurya | thanks mriedem ^ | |
| 16:43:15 | mriedem | np | |
| 16:57:49 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Fup for the bandwidth series https://review.openstack.org/639159 | |
| 16:58:10 | gibi | mriedem: your comments for the bandwidth series are fixed in ^^ | |
| 16:58:54 | gibi | jaypipes, efried: thanks for the comments in the bandwidth series, I will update https://review.openstack.org/639159 with your comments as well, possibly tomorrow | |
| 17:00:44 | artom | Object equality in tests in a PITA | |
| 17:00:48 | artom | *is | |
| 17:01:11 | artom | Expect call: Flavor(<some stuff>), Actual call: Flavor(<exact same stuff>) | |
| 17:03:49 | artom | *facepalm* | |
| 17:03:54 | artom | No, that's not i :( | |
| 17:03:58 | artom | *it | |
| 17:04:46 | sean-k-mooney | stephenfin: left some comments on you os-vif docs changes most are minor | |
| 17:07:24 | sean-k-mooney | artom: object equality check pending change so you need to reset changes on both object before comparing them | |
| 17:08:06 | sean-k-mooney | otherwise you get X != X issues | |
| 17:10:23 | jaypipes | artom: what sean-k-mooney said is almost always the problem with that. | |
| 17:10:54 | sean-k-mooney | jaypipes: artom we proably should just create a function in the base tescae for comparing objects | |
| 17:11:13 | jaypipes | sean-k-mooney: there was one somewhere I think... maybe dansmith can remember :) | |
| 17:11:22 | sean-k-mooney | e.g. self.assertObjEquals | |
| 17:12:30 | sean-k-mooney | jaypipes: would it break the world if we changed __eq__ in base ovo to ignore the changed state of fields? | |
| 17:12:46 | sean-k-mooney | i assume yes since we have not done so before | |
| 17:12:50 | artom | sean-k-mooney, jaypipes, yeah, so it this case it was dumber than that - my main problem was I had an - and an _ in my fake values | |
| 17:13:02 | sean-k-mooney | oh :) | |
| 17:13:11 | artom | But once that hurdle was over, there was still an object I had to replace with a fake string. | |
| 17:13:45 | artom | But yeah, a more intelligent way of comparing objects would be super | |
| 17:13:46 | jaypipes | hah :) | |
| 17:14:18 | artom | It wouldn't even be that hard - implement __eq__ in the base fields, and then recursively compare fields | |
| 17:15:13 | sean-k-mooney | artom: yes but we dont know if anyting depens on the fact that object comparisons current check the changed fields state of the objects | |
| 17:15:34 | sean-k-mooney | os its not that its hard to do but would it break anything | |
| 17:17:03 | sean-k-mooney | artom: https://github.com/openstack/nova/blob/eb5bdd33052166e4375f924456438f11be03310a/nova/test.py#L704 we have this by the way | |
| 17:17:37 | artom | sean-k-mooney, that's not useful when you're asseting call param tho | |
| 17:17:50 | artom | Anyways, it's a pain, but all things considered a minor one | |
| 17:33:22 | mriedem | artom: i've got some object equality test utils in my cross-cell resize series, sec | |
| 17:36:51 | mriedem | artom: https://review.openstack.org/#/c/627892/15/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py@193 | |
| 17:44:03 | mriedem | dansmith: random question, when detaching the root volume of a server and attaching a new root volume, would you expect the device name on that bdm to change? or remain vda or whatever? | |
| 17:44:12 | mriedem | i would expect it to *not* change | |
| 17:44:18 | mriedem | since the boot_index is still 0 | |
| 17:44:32 | mriedem | and the disk_bus and device_type can't change | |