Re: [DISCUSS] Refactoring

2018-05-30 Thread Michael Miklavcic
I'm fine with all the above. Allowing for notes of justification to assist reviewers seems like a good way for us to avoid being pedantic about it. This is fairly subjective, after all. On Wed, May 30, 2018 at 9:59 AM, Casey Stella wrote: > Yeah, that's true. > > On Wed, May 30, 2018 at 8:58 AM

Re: [DISCUSS] Refactoring

2018-05-30 Thread Casey Stella
Yeah, that's true. On Wed, May 30, 2018 at 8:58 AM Otto Fowler wrote: > We can say that any refactoring that *is* necessary, needs to be written > out and justified in the review. > So, we don’t recommend it, but if you have to, and you can reasonably > defend it, OK. > > > On May 30, 2018 at

Re: [DISCUSS] Refactoring

2018-05-30 Thread Otto Fowler
We can say that any refactoring that *is* necessary, needs to be written out and justified in the review. So, we don’t recommend it, but if you have to, and you can reasonably defend it, OK. On May 30, 2018 at 11:53:51, Casey Stella (ceste...@gmail.com) wrote: Yep, I think we can, mike. Let me

Re: [DISCUSS] Refactoring

2018-05-30 Thread Casey Stella
Yep, I think we can, mike. Let me start with a emendation: "Don’t combine code changes with lots of edits of whitespace, comments, or code changes specifically for cosmetic refactoring purposes aimed solely readability; it makes code review and merging difficult. It’s okay to fix an occasional

Re: [DISCUSS] Refactoring

2018-05-30 Thread Michael Miklavcic
Completely agreed on all points. Can we do that here and spin up a vote thread following with the final proposed changes? On Wed, May 30, 2018 at 9:46 AM, Casey Stella wrote: > I'm torn on this, honestly. I completely agree that cosmetic refactoring > gets in the way of review and the risk can

Re: [DISCUSS] Refactoring

2018-05-30 Thread Casey Stella
I'm torn on this, honestly. I completely agree that cosmetic refactoring gets in the way of review and the risk can be more than the reward, especially in a subtle bit of code. That being said, I'm a big fan of opportunistically refactoring to generalize or correct faulty assumptions. Often, I

Re: [DISCUSS] Refactoring

2018-05-29 Thread Otto Fowler
On top of this, refactoring under another PR’s goals tends to be less documented as to the intent and effect. +1 for the idea, we should have a vote round or edit round on the doc’s specific text. Although I will say, that some things it doesn’t matter how much you break them up wrt reviews. We

[DISCUSS] Refactoring

2018-05-29 Thread Michael Miklavcic
I want to bring up the subject of code refactoring and how we should manage this in PR's as our product evolves. As Metron matures, it's only natural that we'll have and increasing number of contributors, and subsequently contributions affecting many hardened parts of the code base. We've