My style wins!

  • (2 Pages)
  • +
  • 1
  • 2

17 Replies - 2063 Views - Last Post: 13 July 2017 - 02:42 PM Rate Topic: -----

#1 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 5887
  • View blog
  • Posts: 20,095
  • Joined: 05-May 12

My style wins!

Posted 13 June 2017 - 11:20 PM

So I was doing a bunch of code reviews yesterday morning and I noted that a particular developer was not following the coding style that was originally in the file (using 2 space indents instead of 4, no whitespace around identifiers and operators unlike the original which made appropriate use of whitespace, etc.) I left comments saying that I wasn't going to block the merge, but if people decided to take it as is, a technical debt bug should be opened about consistent style in the file. Lo and behold, an hour ago I see the pull request updated. Now the entire file had been converted over to his style. Yeah, so the style is now consistent, but the diffs are now useless to see what changed. This is from somebody with 15 years experience and had worked as a dev lead and a dev architect. WTF?!?!

(Since our architect has "approved" this kind the of change, I am very tempted to change all files to 8 character tabs, and then 3 character tabs depending on odd or even days. It is also very tempting to also do force pushes with history re-writes because apparently, having useful history is not important to this team.)

Is This A Good Question/Topic? 1
  • +

Replies To: My style wins!

#2 Salem_c  Icon User is offline

  • void main'ers are DOOMED
  • member icon

Reputation: 2130
  • View blog
  • Posts: 4,196
  • Joined: 30-May 10

Re: My style wins!

Posted 14 June 2017 - 03:52 AM

Put up a wall chart asking people to add a tick if they've suffered from a merge conflict caused by gratuitous reformatting of code which would otherwise have remained unchanged.

Include how many hours it took to recover from said nonsense.

Discuss at the next sprint retrospective.
Was This Post Helpful? 2
  • +
  • -

#3 jon.kiparsky  Icon User is offline

  • Chinga la migra
  • member icon


Reputation: 10681
  • View blog
  • Posts: 18,291
  • Joined: 19-March 11

Re: My style wins!

Posted 14 June 2017 - 06:21 AM

This story makes me feel all stabby.
Was This Post Helpful? 0
  • +
  • -

#4 xclite  Icon User is offline

  • I wrote you an code
  • member icon


Reputation: 1237
  • View blog
  • Posts: 4,028
  • Joined: 12-May 09

Re: My style wins!

Posted 14 June 2017 - 07:11 AM

The Go language gets this right with a standard formatting tool. I think in reality the best way to avoid this is to just put a style checker into a project every time you start one. If I have to pick between consistent style and getting my style, I'll go with consistency.
Was This Post Helpful? 1
  • +
  • -

#5 Atli  Icon User is offline

  • Enhance Your Calm
  • member icon

Reputation: 4238
  • View blog
  • Posts: 7,216
  • Joined: 08-June 10

Re: My style wins!

Posted 14 June 2017 - 07:14 AM

Yea. I prefer putting a coding style guide into the project documentation, and then fire anybody who can't be bothered paying attention.
Was This Post Helpful? 0
  • +
  • -

#6 jon.kiparsky  Icon User is offline

  • Chinga la migra
  • member icon


Reputation: 10681
  • View blog
  • Posts: 18,291
  • Joined: 19-March 11

Re: My style wins!

Posted 14 June 2017 - 08:26 AM

On our project, we use a simple alias that runs flake8 and pep8 (python style checker and linter) against the files that have changed relative to the development branch and shows you the lines that are in conflict with existing standards. The rule is, if this isn't clean, you're not done. The main remaining problem is that we don't have a good way to do style checking on our templates, so we have to check those manually, but we have pretty well-defined standards on those and people are generally good about sticking to those. (especially since I've made sure that the templates that I've touched in the last year are all basically compliant with our standards, so if someone mucks them up, it stands out :) )
Was This Post Helpful? 0
  • +
  • -

#7 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 5887
  • View blog
  • Posts: 20,095
  • Joined: 05-May 12

Re: My style wins!

Posted 14 June 2017 - 07:27 PM

I need a drink. So after 3-4 commits in those files with his style, he then reformatted the entire file back to 4 char indents (but kept the whitespace only where necessary) and then expected another code review. I washed my hands of it.
Was This Post Helpful? 0
  • +
  • -

#8 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 5887
  • View blog
  • Posts: 20,095
  • Joined: 05-May 12

Re: My style wins!

Posted 03 July 2017 - 11:31 AM

Another follow up to this, so the our serial style changer has managed to screw over somebody else by inducing a merge conflict. I pointed out on our Friday meeting this is exactly why one does not modify lines of code that they are not changing. The merge conflict continues to be unresolved 4 days later and still counting. I wish it was not an innocent bystander who got hit, but I'm not going to step in to resolve the merge conflict for them because I want this to be an object lesson. Yes, I do feel bad that the victim had only a two line change that he now cannot merge in, but I'm going to stick to my guns.
Was This Post Helpful? 0
  • +
  • -

#9 jon.kiparsky  Icon User is offline

  • Chinga la migra
  • member icon


Reputation: 10681
  • View blog
  • Posts: 18,291
  • Joined: 19-March 11

Re: My style wins!

Posted 03 July 2017 - 12:19 PM

We handle this with a pretty strict adherence to PEP-8 - if your code doesn't pass flake8, it doesn't get merged.
This means that old code usually has to get cleaned up, but once it's cleaned up it stays cleaned. And it also means there's not a lot of room for arguments. (though oddly enough people still find ways to get arguments to happen)
Was This Post Helpful? 0
  • +
  • -

#10 xclite  Icon User is offline

  • I wrote you an code
  • member icon


Reputation: 1237
  • View blog
  • Posts: 4,028
  • Joined: 12-May 09

Re: My style wins!

Posted 03 July 2017 - 12:21 PM

Had a teammate reformat an entire file that I had a standing PR against the other day. I think he just picked it and decided to fix it. On the one hand I'm all about fixing the formatting of files, as we're enforcing a style post-prototype-which-became-the-product, I just wish he'd do it right at the end or beginning of a release.
Was This Post Helpful? 0
  • +
  • -

#11 Skydiver  Icon User is online

  • Code herder
  • member icon

Reputation: 5887
  • View blog
  • Posts: 20,095
  • Joined: 05-May 12

Re: My style wins!

Posted 04 July 2017 - 05:24 PM

The guy with the two line change begged me to resolve the merge conflict. I stepped in and helped because the serial style changer changed his style yet again, and had two PRs pending. Perfect time to set him up to be on the loosing end of not just one, but two merge conflicts.

Even better, he won't be able to do a simple "accept mine" without breaking our two line changer's code. For him to plea that it was due to a bad merge will not hold water because with his 15 years experience, he should know better than to just merge without reviewing and testing. And to simply just "accept theirs" will undo all his restyling and new code as well as not be consistent with the other files he is committing with his latest new style.

Mischief accomplished!
Was This Post Helpful? 4
  • +
  • -

#12 NeoTifa  Icon User is offline

  • NeoTifa Codebreaker, the Scourge of Devtester
  • member icon





Reputation: 4081
  • View blog
  • Posts: 18,152
  • Joined: 24-September 08

Re: My style wins!

Posted 07 July 2017 - 06:40 AM

I've done this once, and I felt terrible. The code was so badly formatted I couldn't see where anything was, so I just clicked the "format code" button, and magically I could do stuff, but the code reviewer bitched me out something fierce.
Was This Post Helpful? 0
  • +
  • -

#13 jon.kiparsky  Icon User is offline

  • Chinga la migra
  • member icon


Reputation: 10681
  • View blog
  • Posts: 18,291
  • Joined: 19-March 11

Re: My style wins!

Posted 07 July 2017 - 07:24 AM

What is this with people not having a house style guide and running a linter? Why wouldn't you do this?
Was This Post Helpful? 0
  • +
  • -

#14 NeoTifa  Icon User is offline

  • NeoTifa Codebreaker, the Scourge of Devtester
  • member icon





Reputation: 4081
  • View blog
  • Posts: 18,152
  • Joined: 24-September 08

Re: My style wins!

Posted 07 July 2017 - 07:29 AM

Idk. We have one, but nobody knows truly where it is, and these formatting issues are caused by shitty contractors who cheated their way through school and can't program their way out of a paper bag. \_(ツ)_/
Was This Post Helpful? 0
  • +
  • -

#15 jon.kiparsky  Icon User is offline

  • Chinga la migra
  • member icon


Reputation: 10681
  • View blog
  • Posts: 18,291
  • Joined: 19-March 11

Re: My style wins!

Posted 07 July 2017 - 08:21 AM

And who put the contractor in a paper bag in the first place?
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2