| *** gcb has quit IRC | 01:33 | |
| *** gcb has joined #openstack-tc | 01:34 | |
| *** jpich has joined #openstack-tc | 06:59 | |
| *** dtantsur|afk is now known as dtantsur | 07:46 | |
| *** cdent has joined #openstack-tc | 08:48 | |
| *** cdent has quit IRC | 09:04 | |
| *** cdent has joined #openstack-tc | 09:05 | |
| *** cdent has quit IRC | 09:26 | |
| *** cdent has joined #openstack-tc | 10:02 | |
| *** dtantsur is now known as dtantsur|brb | 10:14 | |
| *** cdent has quit IRC | 11:08 | |
| *** cdent has joined #openstack-tc | 12:28 | |
| *** dtantsur|brb is now known as dtantsur | 13:00 | |
| *** hongbin has joined #openstack-tc | 14:08 | |
| *** marst has joined #openstack-tc | 14:22 | |
| * smcginnis flips on the light and puts out the sign | 15:00 | |
| * cdent passes a glass of ice cold lemonade smcginnis | 15:00 | |
| cdent | there’s more here for anyone else around | 15:01 |
|---|---|---|
| * johnthetubaguy raises his hand | 15:01 | |
| * cdent lemonades johnthetubaguy | 15:04 | |
| cdent | if there’s nothing else, I’ll (re-)point people at https://review.openstack.org/#/c/496404/ | 15:05 |
| cdent | and ask: how do we make that go? | 15:05 |
| mugsie | I would have concerns about it being something that is rewarded | 15:08 |
| cdent | “it” == cleaning things up? | 15:08 |
| mugsie | the last thing I want to see is a ton of reviews breaking up long files, just so that a company say the worked on a top 5 | 15:09 |
| mugsie | it depends on the level of cleaning up - people tend to take what nova / keystone etc do, and just try to apply it blindly to other projects | 15:10 |
| cdent | that’s why I’m trying to be circumspect about metrics because I think they point to gaming | 15:10 |
| mugsie | yeah, and sources of endless bikesheding | 15:10 |
| dhellmann | o/ | 15:11 |
| cdent | On the other hand, if we have fewer really long files, I’d be pretty happy. | 15:11 |
| dhellmann | cdent : I'm finally getting around to reading https://medium.com/@mikeal/community-imbalance-theory-c5f8688ae352 -- good stuff, thanks for sharing that link | 15:11 |
| cdent | dhellmann: ah, good, glad you like it. I think it’s at least a useful perspective, and at best, has some good framework for thinking about stuff | 15:11 |
| dhellmann | ++ | 15:12 |
| mugsie | sure. we have one really long one, and it annoys me. but, to break that up well, takes effort, and planning, and someone who knows the project | 15:12 |
| dhellmann | cdent, mugsie : on the simplifying thing, I wonder if the right approach here is to ask teams to provide a list of changes they have been trying to get to for a while but haven't quite made it, and measure the burndown against that list | 15:13 |
| dhellmann | like gcb is doing in oslo, for example | 15:13 |
| smcginnis | I think there are a lot of folks that do think "we need to simplify things" without a clear idea of what or how. | 15:13 |
| dhellmann | yes, and we've seen how easily we can end up with a flood of mechanically generated patches | 15:14 |
| smcginnis | But maybe if we provided a common wiki page or etherpad that various projects could list some efforts they'd like worked on, it could be a good way for folks looking for something to tackle to get directed to something useful. | 15:14 |
| smcginnis | Other than fixing spelling and changing http to https everywhere. | 15:14 |
| dhellmann | or removing utf-8 headers | 15:15 |
| mugsie | dhellmann: ++ | 15:15 |
| mugsie | I did like the oslo cleanup stuff | 15:15 |
| cdent | Not to put too fine a point on it, but I think part of the issue here is that projects aren’t concerned about some of this stuff, and it makes the project less open to new people. So relying on the projects to declare where the work should happen might be a non-starter | 15:15 |
| smcginnis | Yeah, for some projects, I do think that would be the case. | 15:16 |
| dhellmann | maybe. otoh, if the projects don't buy in to the idea of doing this, they'll be less likely to review patches with a positive approach | 15:16 |
| mugsie | cdent: sure. but project that have small review teams see a lot of pointless patches that we still have to review, so they get jaded | 15:16 |
| dhellmann | so maybe we don't want to say all projects, but just have some way for projects to indicate interest in signing up | 15:16 |
| dhellmann | mugsie : even some of the larger teams have that reaction | 15:17 |
| cdent | mugsie: yeah, I totally get that it is a mixed bag and there are many conflicting factors | 15:17 |
| dhellmann | so we need a way to channel contributor energy in a direction that each team finds useful | 15:17 |
| dhellmann | like I suspect the rosmaita and the glance team wouldn't want a lot of code churn for "simplification", unless the contributor was also actively working on some of the other priorities or interested in becoming a core reviewer | 15:18 |
| smcginnis | I was thinking of the glance/glare case. | 15:18 |
| smcginnis | If part of the motivation for glare is that the glance code base is overly complicated, I wonder if it would be faster migrating to a new service vs actually spending the time to simplify the architecture and code of glance. | 15:19 |
| johnthetubaguy | smcginnis: for me the problem is APIs and our user base | 15:20 |
| smcginnis | johnthetubaguy: Definitely. So I'm thinking of that effort of migrating users and projects to potentially a new API, and how long that transition would take, compared to just making glance better. | 15:21 |
| smcginnis | Granted, there are other things in glare, but I don't really want to get into that discussion right now. :) | 15:22 |
| johnthetubaguy | smcginnis: yeah, its a different discussion, I know in Nova we landed on infinity (in the sense of some time after most of the current contributors would have stopped working on the project) | 15:23 |
| cdent | It may be that coming up with a systemized way of doing cleanups is the wrong approach. The real problem is that we don’t have a culture of rigorous maintainability (using generally accepted rules of thumb) nor of refactoring | 15:23 |
| johnthetubaguy | as in infinity is the time needed to migrate | 15:23 |
| smcginnis | johnthetubaguy: I think that may be accurate. | 15:23 |
| cdent | to reach back to the article dhellmann linked, we have become unbalance by urgency | 15:23 |
| dhellmann | cdent : there is some of that. there's also a real impact on existing contributors when the code base undergoes large changes with some frequency | 15:25 |
| dhellmann | whether that applies depends on the nature of the simplification, of course | 15:25 |
| dhellmann | and I guess it's not even "existing" contributors if changes cause a lot of rebases for other ongoing work | 15:26 |
| smcginnis | dhellmann: That's an issue I've seen. Reorganzing things usally conflicts with other in-flight work. | 15:27 |
| dhellmann | cdent : I saw that article applying more to the "unmet requirements" discussions than to simplification, but I think you're right that there's some overlap | 15:27 |
| cdent | that’s perhaps why a cultural shift (so it is more piecemeal) is more important than a set of goals | 15:27 |
| dhellmann | smcginnis : right | 15:27 |
| cdent | but at least for me, I’d gladly pay a bunch of rebases and code shift to eventually be able to spend less time disentangling a method that is 100s of lines long and has huge numbers of collaborators | 15:28 |
| cdent | I’m _exhausted_ | 15:28 |
| dhellmann | how many cases like that do you think we have? I honestly couldn't say, myself. | 15:29 |
| johnthetubaguy | the problem I have had is confidence against regression when reviewing the refactoring, I have been on both ends of that really | 15:29 |
| dhellmann | is it a small enough number to make a list? | 15:29 |
| smcginnis | The difficulty I've seen is I know of a lot of people that want to simplify things, but either don't have the time themselves, don't know where to begin, or can't get past that hurdle of causing conflicts. | 15:30 |
| johnthetubaguy | the problem I had is finding folks with time to review the changes before the conflicted into needing to be thrown away | 15:31 |
| cdent | that’s an issue for changes in general | 15:31 |
| cdent | and one which needs to be generally resolved | 15:32 |
| dhellmann | smcginnis : that's another thing we need to start doing: having our more experienced contributors spend more time teaching new folks than they do now (and maybe more time than writing code themselves) | 15:32 |
| johnthetubaguy | I like the idea of having some regular way to highlight these things, making the list is probably the best way to find out if that would work | 15:32 |
| smcginnis | I'm generally against metrics driven development, but maybe as a start we try to use something like radon. There could be a lot of gaming, but I think eventually if those numbers get better, it could result in easier to maintain code. | 15:32 |
| smcginnis | dhellmann: ++ | 15:32 |
| johnthetubaguy | cdent: true, its a more general problem | 15:32 |
| smcginnis | Radon just being one tool that came up in a quick search. | 15:32 |
| cdent | smcginnis: yeah, radon is the one I named simply because it is the top of the search results and one I’ve used before | 15:33 |
| smcginnis | Might be worth just adding not voting jobs with that output so those interested can take a look and see if numbers are getting better or worse. | 15:33 |
| dhellmann | smcginnis : I agree we need to be careful with metrics, that's why I proposed a specific list rather than measurements | 15:33 |
| cdent | I actually don’t have a lot of faith in its results. They are useful to find some stuff, but humans are better. | 15:33 |
| dhellmann | yeah, this feels like the sort of thing where we *know* where the problem points are, or are likely to be | 15:33 |
| smcginnis | I think there probably isn't one good answer to this problem. | 15:33 |
| mugsie | yeah, we ran sonar against designate (and I think octavia did as well for a while) and it did not really add anything useful | 15:34 |
| smcginnis | As far as a top-5 item, I would like to see a specific list. | 15:34 |
| smcginnis | As far as ongoing and building this into our development culture, I think we need something like the radon output to give us feedback. | 15:34 |
| johnthetubaguy | hmm, do we want to restrict things in that way? highlighting the issue could help us find a solution | 15:35 |
| dhellmann | do we have a tool that produces good feedback then? radon is better than sonar? | 15:35 |
| * smcginnis smells a PTG discussion | 15:35 | |
| dhellmann | can someone give me a link to radon? I'm just finding stuff about ventilating basements. | 15:36 |
| cdent | http://radon.readthedocs.io/en/latest/index.html | 15:36 |
| dhellmann | thanks | 15:36 |
| dhellmann | oh, it has a flake8 plugin, so we may not need a new job | 15:37 |
| smcginnis | Just noticed they point to xenon for CI jobs: https://github.com/rubik/xenon | 15:37 |
| cdent | the radon plugin against nova is somewhat slow, but is nicely informative | 15:42 |
| smcginnis | Just ran against Cinder. Think this will take a little while to grep through. | 15:42 |
| dhellmann | ok, Radon says this function is too complex https://github.com/dhellmann/whereto/blob/master/whereto/app.py#L27 | 15:43 |
| dhellmann | it has 1 loop, and 1 conditional | 15:43 |
| dhellmann | unless I'm misreading the output? | 15:43 |
| smcginnis | Two conditionals. That's way too complex. :D | 15:44 |
| dhellmann | http://paste.openstack.org/show/619337/ | 15:44 |
| dhellmann | ah, 2, yes, I can't count | 15:44 |
| cdent | no, dhellmann, you got an A for that method | 15:44 |
| cdent | main got a C | 15:44 |
| dhellmann | oh, what is the F in the left column? | 15:45 |
| cdent | function | 15:45 |
| dhellmann | ah | 15:45 |
| dhellmann | I thought that was the score | 15:45 |
| dhellmann | ok, well that makes me feel a bit better :-) | 15:45 |
| cdent | yeah, it is disorienting | 15:45 |
| cdent | if you use the flake8 plugin, it only shows you poor output | 15:45 |
| smcginnis | The output is definitely not immediately intuitove. | 15:45 |
| cdent | so can be a bit less alarming | 15:45 |
| * dhellmann avoids pointing out the irony of a complexity measuring tool having complex output | 15:46 | |
| smcginnis | You can also use -nc to only show a C ranking or worse. | 15:46 |
| smcginnis | Haha, true. | 15:46 |
| smcginnis | Simplicity is complex. | 15:46 |
| dhellmann | so main() got a C. What would you change? | 15:46 |
| cdent | make it do less, extract some stuff`/ | 15:48 |
| smcginnis | Maybe move out the reading of rulesets and tests into separate calls? This is where it would get tricky. What's really an improvement vs making something harder to read for the sake of better number. | 15:48 |
| dhellmann | right | 15:48 |
| smcginnis | Maybe just D or worse to start? | 15:48 |
| dhellmann | I guess the stuff after the file reading could be split out, but it's just printing results | 15:49 |
| cdent | handle_mismatches(), handle_untested(), etc | 15:49 |
| cdent | I know I’m a blind grape in the path of progress, but I tend to think if I can’t see the entire method in my 80x24 vt220 it’s too big | 15:51 |
| cdent | 277 “errors” in nova (using 10 as the score) | 15:53 |
| dhellmann | I guess I tend to use whitespace between stanzas as a replacement for splitting up medium length methods into smaller ones when there is no real thematic difference or reusability | 15:55 |
| cdent | I also love vertical whitespace. that ends up meaning either a lot of methods that violate my vt220 or a lot of private methods | 15:56 |
| * cdent doesn’t really use a vt200, just to be clear | 15:56 | |
| smcginnis | vt100? | 15:57 |
| cdent | teletype | 15:57 |
| smcginnis | I like the mi (maintainability index) but less specific feedback. | 15:58 |
| dims | is this "we need this, come help us do it" to folks who are not currently engaged/active? | 16:00 |
| cdent | dims: probably not, which is part of why it’s not super workable as a top-5, but it seems a way to get the ball rolling | 16:01 |
| dims | if they do show up, how do we help them help us? (given our review bandwidth) | 16:01 |
| smcginnis | Ideally it would be good to have the already active folks doing some of the bigger refactoring, but I could see some of it bing "low hanging fruit". | 16:01 |
| * cdent has to go to api-sig meeting | 16:02 | |
| cdent | thanks for thinking about this stuff | 16:02 |
| *** openstackgerrit has quit IRC | 16:04 | |
| smcginnis | radon raw output is a lot of data, but interesting. Could just target the ones there where "Comments: 0" | 16:08 |
| * cdent is pleased that he has helped give smcginnis a new way to see parts of the world | 16:09 | |
| smcginnis | :) | 16:09 |
| dims | smcginnis : paste me what you are looking at please? :) | 16:28 |
| *** jpich has quit IRC | 16:29 | |
| *** dtantsur is now known as dtantsur|afk | 16:47 | |
| *** cdent has quit IRC | 17:04 | |
| smcginnis | dims: Sorry for the delay. This is what I was looking at: http://paste.openstack.org/show/619347/ | 17:55 |
| dims | ah thanks smcginnis | 17:56 |
| smcginnis | Thinking the percentage numbers comparing comments to code might be an interesting metric. At least to start identifying areas where maybe there wasn't enough done. | 17:56 |
| *** openstackgerrit has joined #openstack-tc | 19:15 | |
| openstackgerrit | Brian Rosmaita proposed openstack/project-team-guide master: Add "How to Preview Release Notes at RC-time" https://review.openstack.org/497589 | 19:15 |
| *** marst has quit IRC | 23:05 | |
| *** hongbin has quit IRC | 23:23 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!