[wmil]

It depends what you mean by code snob... I've had a dev replace large section of my code with "cleaner" code that didn't work in all cases, comment out the failing tests, then push that.

Some people are convinced that making components isolated and aesthetically clean is more important than making them functional.

[logicallee]

>I've had a dev replace large section of my code with "cleaner" code that didn't work in all cases, comment out the failing tests, then push that.

This is really interesting. Can you elaborate on what they might have been thinking when commenting out those tests? (which seems pretty blatant). Were the tests commented as to what they were doing, and obviously were important, relevant, and should have passed?

My impression is that when you go to a higher (cleaner) level of abstraction, you lose some immediate abilities. For example if you move a global variable without classes to a static variable in the class it really belongs with (after writing that class), you can never (by doing that) enable additional functionality over having it be a global variable. But the variable might no longer be accessible at some point, perhaps if a different base class has a static member whose initialization depends on another one already being initialized - that sort of thing.

which is the whole point. you are applying a new abstraction, and in the new abstraction they're not global variables but properties of a class. Perhaps some tests become irrelevant or need to be rewritten.

So, you could have some test fail as written, because you've moved to a higher level of abstraction. This makes me wonder what the exact tests were in the case you refer to...

[nightski]

Well it would likely be better to have a discussion about it then? Instead of a brewing conflict among members of the team? It's also where leadership comes in. Someone needs to make a decision and guide the team in a particular direction. The most important thing is not that he/she chooses the "best" way (implying one engineer was "wrong") but rather simply that a way is chosen and everyone follows it.

[RogerL]

I have to disagree, no bugs is better than some bugs. Pushing a change that introduces bugs is wrong (without the scare quotes).

This (sw design) is a multidimensional optimization problem with a somewhat vague cost function. But argmin on the # of bugs and argmax on feature completion is far more important than everyone following some random standard.

[ultrasaurus]

There's a pretty large distinction between reviewers that are manually linting the code base and ones who change code to be easier to read but less working :)