Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Reformatting 100k Files at Google in 2011

laurent.le-brun.eu

Post reply on HN
Print view View original

71–80 of 162 posts

Re: Reformatting 100k Files at Google in 2011

#71

Post by diegocg »

rsc wrote: post #16
flymasterv wrote: post #12

I don’t understand why they had to format 100k files. You enforce the format with a presubmit and let the code get formatted in the next change. I have long felt that Google’s strength has always been making a bad architectural choice and then executing on it flawlessly. So many systems are designed in ways that require incredible technical execution to make them workable, and they do it.

If you do it that way, then what should be a 1-line BUILD file change turns into something that changes every line. It distracts from the actual purpose of the future change. Many directories aren't touched for long periods of time. A few months from now someone tries to make a 1-line change and is unpleasantly surprised they have to deal with tons of seemingly spurious formatting changes. Not good. Putting the time…

Make the one line change be a commit, then the reformatting be another one, review only the first one. It shouldn't be a problem with a proper review system.

Re: Reformatting 100k Files at Google in 2011

#72

Post by montag »

Arainach wrote: post #66
rapfaria wrote: post #64

After reformatting, did git blame always pointed to those commits and the most recent author, or were they added to --ignore-rev?

Google doesn't use Git internally, and its code search and source control tools expose the concept of "show blame before this change", so in practice reformats like this aren't troublesome with regard to blame.

This is true but I think it would still be nice for VCS to have a first class concept of "peek through" changes (whitespace, formatting, etc.) for the purpose of blame.

Re: Reformatting 100k Files at Google in 2011

#73

Post by dilyevsky »

diegocg wrote: post #71
rsc wrote: post #16

Earlier quoted context omitted.

If you do it that way, then what should be a 1-line BUILD file change turns into something that changes every line. It distracts from the actual purpose of the future change. Many directories aren't touched for long periods of time. A few months from now someone tries to make a 1-line change and is unpleasantly surprised they have to deal with tons of seemingly spurious formatting changes. Not good. Putting the time…

Make the one line change be a commit, then the reformatting be another one, review only the first one. It shouldn't be a problem with a proper review system.

Googles source control system at the time (Perforce) didn’t allow for this at least easily. Not sure about now

Re: Reformatting 100k Files at Google in 2011

#75

Post by tantalor »

diegocg wrote: post #71
rsc wrote: post #16

Earlier quoted context omitted.

If you do it that way, then what should be a 1-line BUILD file change turns into something that changes every line. It distracts from the actual purpose of the future change. Many directories aren't touched for long periods of time. A few months from now someone tries to make a 1-line change and is unpleasantly surprised they have to deal with tons of seemingly spurious formatting changes. Not good. Putting the time…

Make the one line change be a commit, then the reformatting be another one, review only the first one. It shouldn't be a problem with a proper review system.

All changes need to be reviewed. That's the point of code reviews.

Your suggestion would allow people to bypass the code review by just saying "oh it's just cleanup don't worry".

Re: Reformatting 100k Files at Google in 2011

#76

Post by mckn1ght »

tantalor wrote: post #74
rapfaria wrote: post #64

After reformatting, did git blame always pointed to those commits and the most recent author, or were they added to --ignore-rev?

Blame can be configured to ignore cleanups like this.

This has always been something I use to argue against just reformatting code in isolation. TIL git provides a way to ignore that. Thanks!

Re: Reformatting 100k Files at Google in 2011

#77

Post by ammar2 »

haburka wrote: post #67
rsc wrote: post #19

My notes say it was 193k at the start. The final dashboard when we stopped said "216,626 / 216,890 = 99.8%; 264 to go". The other correction I would make is that this post does not mention Nilton Volpato, who had written an earlier Buildifier and graciously accepted replacing his implementation with a new one and then taking over ownership for that new implementation as well. (Eventually ownership moved to Laurent's…

You didn’t have Rosie to automatically split up your changes and send them out yet?? That must have been rough. LSCs are way easier now

Yeah I'm surprised by that as well. As far as I remember, Rosie started out in 2010 and people were using in 2012. Maybe the clustering/splitting didn't support this use-case or it wasn't well-known enough?

Re: Reformatting 100k Files at Google in 2011

#78

Post by jenadine »

duped wrote: post #69
__s wrote: post #68

Earlier quoted context omitted.

language servers have pushed things to inspecting source code while programmer has partially written code, so having a kind of (invalid-span content="garbage") node in AST helps

Language servers usually are designed around full/concrete syntax trees instead of ASTs for exactly this reason. Adding error nodes to the AST is a hack that hurts more than helps. More technically, language servers usually have a CST that they use to build the AST incrementally, and the AST contains references back to the CST that generated it. This is what allows you to handle incremental text edits and compile sma…

What's wrong with error node in the ast?

I've seen language server that completely ignores the parts with error, and I much prefer error nodes because then I still know there is something and these error node can still have children

Re: Reformatting 100k Files at Google in 2011

#80

Post by mmastrac »

ragall wrote: post #79

The real lesson here should be that source code shouldn't be stored in a text format, but in a well-defined strict binary format that stores the parse tree directly, which completely eliminates the need for formatters.

A well-defined, unambiguous formatting standard for a text file is philosophically identically to a well-defined strict binary format. :)
Post reply on HN