12 ms·
Reformatting 100k Files at Google in 2011
- wonger_ 2y agoAutoformatting is so nice. Crazy to think that formatters only became popular after `gofmt`. I also found this related quote from Russ Cox intriguing: "Most people think that we format Go code with gofmt to make code look nicer or to end debates among team members about program layout. But the most important reason for gofmt is that if an algorithm defines how Go source code is formatted, then programs, like goimports or gorename or go fix, can edit the source code more easily, without introducing spurious formatting changes when writing the code back. This helps you maintain code over time."
- DangitBobby 2y agoThat doesn't really seem like a good reason. They remove all formatting control with an automated tool so it doesn't matter when (if, big if) an automated tool later rewrites some of your source code formatted in a way you don't control?
- Nzen 2y agoMu. Russ Cox quote indicates that the maintainers of goimports and gorename do not need to handle formatting at all (and hence, benefit). The go team put that wholly in the gofmt team's court. Otherwise, the go team would likely have to handle tickets about how the formatting of goimports and gorename differ over time for edge cases (if only by paying the social capital of ignoring the tickets).
- DangitBobby 2y agoThat would fall under what I already assumed to be true, and also under an umbrella they specifically said it wasnt about > end debates among team members about program layout (Except in this case of course it's not "team members") I'm definitely not getting this interpretation from the quote.
- striking 2y agoIt's more common than you think! Tools like https://github.com/facebook/jscodeshift https://github.com/facebook/jscodeshift, https://ts-morph.com/ https://ts-morph.com/, and https://ast-grep.github.io/ https://ast-grep.github.io/ are the ones I'm more familiar with (since my day-to-day is TypeScript). When I led the conversion of a decently-sized codebase from Flow-typed JS to TypeScript, I ensured that a code formatting tool that we were already using on pre-commit and CI called `prettier` was executed after each step. We took a git snapshot of each step of our automated conversion pipeline, and the diff was much clearer with `prettier` in place at each of those steps. We've since used codemods frequently to make huge changes to the codebase in an automatic, reproducible, iterable way. They're very comfortable, very fun, and (thanks to the use of a formatter on all code) rarely produce incomprehensible diffs.
- delichon 2y agoIt reduces cognitive load on integrations, both software and wetware. So many brain hertz are wasted on internally bikeshedding format details. Autoformat fixes a hole where the rain gets in and stops my mind from coding.
- autarch 2y agoThe Perl world had perltidy (first release in 2002) many years before Go was even a thing. It's funny that Perl, a language notorious for it's "There's More Than One Way To Do It" (TMTOWTDI) philosophy had a tidier so early. Of course, perltidy is _ridiculously_ configurable. One thing I really love about gofmt is that it has no configuration at all. I think that was a major "innovation" and I'd love to see more languages adopt this approach.
- XorNot 2y agoThere were also various Python formatters. The problem was the zeitgeist - engineers would tell me that "they didn't want an algorithm messing with their code" as though that was a serious concern, or that wading through endless pull-request changes for syntax was a good use of anyones time.
- ravishi 2y agoThe thing about configurable vs non-configurable in this case is that when its configurable then people will spend time debating how exactly they should configure it.
- autarch 2y agoYes, that's why I love gofmt. There's nothing to debate!
- jrockway 2y agoHelpfully, you can debate between "gofmt", "gofmt -s" and "gofumpt". I actually like how gofumpt formats stuff but ... nobody else on the team would have it, so it would make things worse.
- autarch 2y agoYeah, there's still a few options. But try running `perltidy --help`. Even that doesn't really give you a sense of how many options there are because many of the options are not booleans, but instead take several enum values indicating which style to choose.
- afavour 2y agoI’ve wondered before whether the world would be well served by a programming language (or source control system, I suppose) that just stores ASTs in files rather than text code. When users open the file the editor formats to whatever their personal preference is, then saves edits back to the AST. It really is dumb to be arguing over tabs vs spaces, after all.
- rsc 2y agoAt that point you could even use different languages. Maybe you like programs that look like Lisp and I don't. There was a project at Microsoft Research in the late 1990s/early 2000s that did exactly this - storing ASTs in source control instead of code - but the name escapes me at the moment.
- whstl 2y agoWas it Intentional programming? I remember it being described as something similar to what you say, but the Wikipedia shows something slightly different. https://en.wikipedia.org/wiki/Intentional_programming https://en.wikipedia.org/wiki/Intentional_programming
- refulgentis 2y agoQuick 10 min into Googling, def. double check me: You're right, ex. https://dev.to/jillesvangurp/comment/6gnb/ https://dev.to/jillesvangurp/comment/6gnb/ Sadly there's enough bitrot that ex. intentsoft.com is offline. More here, but article is v opinionated/judgement oriented, comments are useful, but again, bitrot :( https://wiki.c2.com/?IntentionalProgramming https://wiki.c2.com/?IntentionalProgramming It strikes me that what I am describing as "bitrot" may also be "never really shipped, so the vagueness isn't accidental"
- throwaway2037 2y agoHat tip for c2.com link! The follow-up is savage: https://wiki.c2.com/?CritiqueOfIntentionalProgramming https://wiki.c2.com/?CritiqueOfIntentionalProgramming
- omoikane 2y agoAutoformatters got popular was because a lot people don't care about formatting, and those who do care can't win against the auto part of autoformatters. It works for go because gofmt was there from the start, so even if you are returning a multi-dimension array and elements come out unaligned, that's just accepted as how it is and nobody cares. For other languages, people will have to either accept "not caring" as becoming the norm, or actively fight the autoformatter from steamrolling over their code. For people who would give more thought to how their code would be read, autoformatters were often more frustrating than "nice".
- YZF 2y agoInteresting enough people still find things to argue about even with gofmt and goimports. The order of imports and maximum line length are two examples. I imagine if those were done by gofmt/goimports then people would argue about other things. It's why we can't have nice things... Autoformatters do help though.
- freedomben 2y agoYes precisely this. I worked on a team in C++ that vertically aligned things, and at first it seemed like extra work but once I started seriously reading the code, I had an epiphany of realization that it is an amazing improvement in readability, but also makes it super easy to find certain bugs, and also do block editing in tools like vim. After leaving that team, I found that everywhere I tried to introduce this people just shrugged and rolled an auto-formatter over and told me that it's not worth thinking about. IMHO autoformatters are awesome until there's something you care about that the implementers didn't care about, then they are horrible. Problem is, the people who put a lot of thought into those decisions are often in the minority and tend to lose the argument.
- rowanG077 2y agoI never thought it only got popular with go. I remember using formatters before go. What go did do is that it shows how good it is if it is ubiquitous.
- plorkyeran 2y agoI think mandatory code formatting was popularized by Go? Plenty of code formatting tools had been around for quite a while by the time Go came out, but they were the sort of thing you ran on a codebase once to clean up a mess, not something you checked on each commit. A common sentiment was that it was a good idea but was only viable because Go had it from the beginning, which fortunately turned out to be wrong.
- fweimer 2y agoJava IDEs have had optional reformat on safe for a long time. https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Freference%2Fpreferences%2Fjava%2Feditor%2Fref-preferences-save-actions.htm https://help.eclipse.org/latest/index.jsp?topic=%2Forg.eclip...
- jakjak123 2y agoThey have, but the different IDEs do not format equivalent and are way too easy to reconfigure to personal preference.
- deleted 2y ago[deleted]
- vitiral 2y agoI'm going to take a contrarian view here. Code formatting is amazing in a corporate environment where nobody truly cares about their code -- it's just a means to get a paycheck. It's also great for beginners to a language who are still trying to get a handle of the syntax. But where you are nearly the sole owner of a small library and you are crafting that library to be beautiful and understandable... there is something pleasurable about structuring concepts so you have each on a single line, or creating similar functions so the concepts are structured by column. I know not everyone will hold this view and that is fine, but when you are writing your own hobby library in your favorite language for your own purposes I recommend you try it out.
- JZL003 2y agoI do more research, one-off ,code and being able to align equals signs or successive lines which do similar things both makes me happy and makes it easier to skim. I think the Linux kernel does it too. Emacs has a way of aligning by common separators (=/, as well as quick regex) I guess I've never gotten into an actual serious debate with someone over formatting so I don't know what I'm avoiding, but sometimes auto formatted code makes it harder to skim
- lokar 2y agoEverywhere I’ve worked, including Google, many (probably most) people care a lot about the code and the format. Often too much. That was the point rsc was making.
- jsolson 2y ago[flagged]
- neilv 2y agoAlso, sometimes the formatting simply makes reading something messy merely tractable, not aesthetically pleasing. Once, a product launch depended on me urgently kludging a device driver in Python (long story). And this involved a large hand-maintained mapping table. I wrote it quickly but carefully, and found some formatting that made the table readable enough, without implementing a minilanguage in Python. But the Black formatter had been rigged to run automatically on commit, so... poof! :)
- flymasterv 2y agoI 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.
- oceanstone 2y agoEx: Angular
- valicord 2y agoBecause then every commit from now on has a ton of formatting changes that make it harder to see what was actually changed.
- flymasterv 2y agoAnd this is a flaw of Perforce: in a Git/Mercurial system, the presubmit can stack the changes into two commits. In P4, one CL has to contain both changes. And Google uses P4(ish) because monorepo, so they build further abstractions over P4 to enable git and hg in user space which erases most of the potential benefits of either which is also all really, really good software, but it’s all effort necessitated by monorepo. CitC is a work of art, but it is also something necessitated by a stack of other choices that forced their hand into inventing something miraculous to keep hacking around a previous limitation that nobody else has.
- rsc 2y agoMy 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 team.) It looks like it was just under 2,000 commits. We did pretty extensive testing, by having Blaze load a BUILD file and its transitive closure and then dump that parsed form back out to a binary format. Any automated commit had to preserve that parsed-and-dumped binary format bit for bit. The slowest part of the testing was waiting for Blaze to do all the loads. Every day I would prepare and test as many files as I could, break them into CLs (think PRs), mail Rob a shell script he could run to approve them all, and go to bed. Then I'd get up early in the morning (5am ET) to submit the changes, because there were various cached indexes that got updated when BUILD files got submitted, and it seemed better to send them when not many people would be working. That scheme worked until a system did fall over and someone got paged, and then after that I agreed to only submit the large changes during business hours. :-)
- troad 2y agoThe use of light grey text on a darker grey background strains my eyes and makes this unnecessarily unpleasant to read. I'd respectfully suggest increasing the contrast dramatically. I keep a quick little scriptlet in my bookmark bar for cases like this: javascript:(function(){ $('head').append('<style>*{color:#101010 !important; background:#f0f0f0 !important;}</style>'); }()); (A ten second hack job; suggested improvements from front-end friends are welcome.)
- deleted 2y ago[deleted]
- laurentlb 2y agoThanks, I've increased the contrast (not as much as your suggestion, but I hope it's better).
- Dagger2 2y agoI've been using this: javascript:(function() { for (var n of document.querySelectorAll('a, p, li, div')) { n.style.color = (n.nodeName == 'A' ? 'LinkText' : 'CanvasText'); n.style.backgroundColor = 'Canvas'; n.style.font = '500 16px/1.4em sans-serif'; }})(); It uses system colors and thus, if your browser supports them, should adapt to dark mode automatically. Using .style has the advantage that sites can't override the style themselves using .style. (You'd think looping over all these elements would be slow, but it's not.) This version also works on sites that aren't using jQuery, although it wouldn't be hard to use `var s = document.head.appendChild(document.createElement("style")); s.innerText = "...";` for that. I was surprised at how helpful forcing the font face and spacing is. There's a lot of sites out there with bad-looking fonts or huge line spacing on top of unreadably-light gray. I added the background color part based on your version. Thanks for prompting me to try that; the way my bookmarklet didn't work on black backgrounds was occasionally a problem. I also added a bit to force link colors, since neither of our versions handled those well. Perhaps the next step is a "multistage" bookmarklet that applies more rules the more times you click on it, so the more forceful rules (like background color, which often messes up other parts of the site design) can be optional.
- 2y ago
- bsmith0 2y agoI'd be curious what role "global approvers" at Google/Google's scale typically have/how many are there/what's the process?
- dmazzoni 2y agoI was at Google until a few years ago. The purpose of global approvers was exactly things like this. If you want to do a mechanical change to an insanely huge number of files, they can potentially approve it. In my experience, global approvers were used extremely rarely, only in cases like this where the transformation was purely mechanical and it was possible to verify that there were no logic changes. Most of the time rather than global approvers, you were encouraged to use a system that would automatically split your change into a bunch of smaller CLs (PRs), automatically send those to owners of each module, then automatically merge the changes if approved. It would even nag owners daily to please review. If you had trouble getting approval for some files you could escalate to owners of a parent directory, but it'd rarely be necessary to go all the way up to global approvers. Basically if there was even the slightest chance that your change could break something, it's always safer to ask individual code owners to approve the change.
- tylerhou 2y agoEven if you get global approval it is still good to split CLs to avoid e.g. merge conflicts.
- y2mango 2y agoI was one of the global approvers (and also on the Python team until my role at Google was eliminated recently). I no longer have access to the stats, but from my memory, there are currently 50+ global approvers depending on how many are still considered active. Typically, code authors would create a proposal by filling out a doc template. It's usually light weight and also accompanied with examples or full set of the pending code changes. Then 1-3 of us will review and LGTM the proposal. As part of the review, we also determine whether the changes should be sent to local code owners, or "globally approved" by one of us. The default option is to use "global approval", unless the changes need local code owner's knowledge during the code review. Said in another way, when sent to local code owners, their role is not gate keeping the changes, but to provide necessary local knowledge where we as global approvers don't have. Refactoring changes, such as formatting or API migrations, shouldn't bother local code owners because 1) it would just be a waste of their time to review and approve; 2) in practice, we find a central code reviewer for the same large set of code changes is more likely to catch bugs (with review automation tooling) than local reviewers. We consider ourselves as facilitators rather than approvers or gatekeepers of the code changes. Our goal is to make these changes done more efficiently and save engineering time when possible. If you like stats: over the past 5 years, I have reviewed ~300 such proposals and ~40K changelists (equivalent to PRs). One changelist/PR typically contains 10s to 100s of files depending on the nature of the change. When I was most active, I was about ~5th-ish when ranking the number of changes we were approving. There are many global approvers who have approved more than 100K changelists, which is a milestone we celebrate with a cake. Too bad I didn't have the chance to have my cake.
- yegle 2y agoThere are 3 tools that that makes maintaining BUILD files enjoyable: buildifier, buildozer and build_cleaner (internal only unfortunately).
- lopkeny12ko 2y agoWhat is build_cleaner?
- randomifcpfan 2y agoA tool for updating bazel build target dependencies. It inspects build files and source code, then adds/removes dependencies from build targets as needed. It requires using global include paths in C/C++ sources. It is not perfect, but it is pretty nice!
- rsc 2y agoIf you're using Go with Bazel, gazelle is available outside Google: https://github.com/bazelbuild/bazel-gazelle https://github.com/bazelbuild/bazel-gazelle Enabling tools like these was exactly the point of the enforced formatting. It worked extremely well.
- dijit 2y agoI should add that it seems Gazelle is being expanded to other programming languages other than Go. For example: https://github.com/Calsign/gazelle_rust https://github.com/Calsign/gazelle_rust
- rapfaria 2y agoAfter reformatting, did git blame always pointed to those commits and the most recent author, or were they added to --ignore-rev?
- Arainach 2y agoGoogle 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.
- montag 2y agoThis 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.
- tantalor 2y agoBlame can be configured to ignore cleanups like this.
- mckn1ght 2y agoThis has always been something I use to argue against just reformatting code in isolation. TIL git provides a way to ignore that. Thanks!
- ragall 2y agoThe 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.
- mmastrac 2y agoA well-defined, unambiguous formatting standard for a text file is philosophically identically to a well-defined strict binary format. :)
- YZF 2y agoI agree with this sentiment. It also provides an opportunity to store other metadata with the code. It lets different people choose their formatting preference when rendering back to human readable code. The source code is text thing seems like a "legacy" concept. At the very least I think this is an interesting thing to explore. Maybe it leads nowhere...
- jerf 2y agoThe term "bikeshedding" comes up a lot on HN, when people spend a lot of time spinning wheels in endless debates about the things that are easy to debate while indefinitely deferring the tough conversations, but I think an underappreciated aspect of this meme is that the unimportant conversations quite frequently really are just that; unimportant, to everybody involved. Often (but not always) nobody actually cares. Caring is not necessarily represented by the amount of time or text spewed forth on a topic. A lot of things other than passion or importance factor into how much verbiage comes through and it is a mistake to overinterpret mere volume. Unfortunately, the flip side of this coin is harder to deal with; sometimes truly important issues or things that people do deeply care about have disproportionately too little verbiage. Finding those can be very difficult.
- fragmede 2y ago> Caring is not necessarily represented by the amount of time or text spewed forth on a topic. The meta says that it is. there are only 81600 seconds most days, and you get to choose them how you want, so choose how you spend them wisely. if that's arguing over tabs or spaces, then that's your choice.
- rsc 2y agoThis One Trick Will Add 80 Minutes To Your Day
- jerf 2y agoThe meta would say that if people were optimally spending their time on things that matter to them. They don't. If they did bikeshedding wouldn't exist. It obviously does. This is basically the same claim that economics can treat humans as perfectly rational actors perfectly rationally pursuing their perfectly rational goals. It is not a good model of humanity.
- fragmede 2y agoIt's a revealed preference and there's a ton of economic studies about that vs stated preferences. You can say you don't care about tabs vs spaces all you want, but if you spend hours online talking about it, people are going to think you care, no matter what you say. Bringing economics into this, how do you metricize caring? Can you simplify it to be the time and money you put into a thing? "All models are wrong, but some are useful." -G. Box
- aiuto 2y agoI want to start by saying that I do not want to diminish or disparage the work that Russ, Rob, Laurent, and others have done. It has made the Google code base better. That is an inarguable fact. Nor do I want to pick on buildifier or gofmt or any other tool as a singleton problem. I'll talk about buildifier because that is what I personally fight with. Others may have different demons. (YDMV - "your daemon may vary". I'm taking the authorship of that one now, in case it ever takes off). But back to the point... formatting rules without firm, incredibly strict enforcement ends up being a tax on the janitors - the people who clean the code base and do large scale changes. That makes me sad. These are the people who care a great deal about code health, and their work is hindered by the lint checks that we have imposed. Let me give an example. I'm trying to eliminate a constraint in the build system. It's a "small" large change - only O(30K) instances. (Yeah, Google scale is different). I have an incredible wealth of tools available to me to automate the process. For the benefit of the Googlers, I can identify Blaze targets to change, use buildozer to fix them, and ship off CLs to review. But the changes I want to make are often ones which should be reviewed by the code owners, and not globally approved. So possibly O(10K) individuals might be involved in reviews. Let's explore the problem. First, shouts to y2mango for bringing up incremental formatters. This should be the default for all tools. And another to flymasterv for raising the question of "why not just format as each person touches a BUILD file". Here's the situation. 1. buildozer is really good at rewriting BUILD files syntactically correctly. 2. It has an unfortunate side effect of not being incremental. It calls buildifier to rewrite the entire file. 3. We update the formatting rules to make them stricter over time. That means that a "correct" BUILD file on January 1, might require changes on March 1. 4. Buildifier findings are advisory, rather than mandatory. 5. No team is staffed with repeating the monumental work this post started with. The reality on the ground is that little touched BUILD files become stale, and would require a formatting update over time. It is actually worse than that, because many teams take the path of ignoring buildifier warnings and committing their working code anyway. Without continual BUILD file reformatting there is a lot of stale floating around. [Root cause: We could fix this by promoting people for doing that repeat work. But we don't. We promote for the initial sprint.] And then a janitor comes along. I use bulldozer to fix a problem. It reformats an ancient BUILD file completely (not incremental). I send it to the code owner. They see changes far beyond my 2 line fix. They reject it, or ask for a change to only the two lines that actually mattered. Sure. I can hand build the change once or twice. But not for a few hundred, or thousands files. So.... I have to hack up an incremental format. Or, it turns out that users are very happy if I don't bother with formatting at all, and just change single lines. It's not that any individual is right or wrong. It is that they all have a choice and a preference and Google created a policy that allowed individual teams to have a choice of strict compliance or not. That is the failure. If you are going to have a policy about code formatting: - make it hard mandatory for everything except a "break glass" situation - if the policy can evolve, staff a team with enforcing it globally The fact that Google, as a company, does not reward this behavior does not take away from any individual's accomplishments. This post may sound grumpy to an outsider, but I am constantly amazed at the tools I have available to fix things on an enormous scale. The friction is usually only where we have good intentions, without the policy teeth to enforce alignment with the intentions. That's a management problem, not a technical one.
- vasco 2y agoSounds like a quick way to get Golang installed on every Google developer machine and drive adoption.
- rsc 2y agoNo, because Go compiles to binaries, like C/C++ but unlike Java or Python. Only the buildifier binary needed to be installed on everyone's machine. (I think Go was already installed on every Google developer machine at that point anyway.)