Categories
Uncategorized

2021W39

Much of my time this week was taken up with a migration. We have a library internally at Google which does exactly what the new UnixMicro() and UnixMilli() in the time package does (but was available many years prior to 1.17). Initially, I started writing a “tip of the week” article about how they can be used. This quickly morphed into a “migrate everyone to the new functions” project. Thankfully, having everyone in a large monorepo helps enormously. I’ve now submitted 96% of the migration changes a couple of days after starting (using the large-scale-change aka LSC process).

Making the initial change was surprisingly simple, using a combination of codesearch and gofmt -r. I was lucky that I didn’t need anything more involved; however, I see that Eli Bendersky has an excellent introduction, Rewriting Go source code with AST tooling which would cover more advanced needs.

codesearch -l 'unixtime\.(To|From)(Micros|Millis)'
while read filename
do
  gofmt -r 'unixtime.ToMicros(a) -> a.UnixMicro()' "$filename" |
  gofmt -r 'unixtime.FromMicros(a) -> time.UnixMicro(a)' |
  gofmt -r 'unixtime.ToMilli(a) -> a.UnixMilli()' |
  gofmt -r 'unixtime.FromMillis(a) -> time.UnixMilli(a)' |
  goimports > "$filename.tmp" &&
  mv "$filename.tmp" "$filename"
done

I ended up with a change touching around 4100 files. The LSC tools split this up into around 250 changes, and I worked with an approver to get these reviewed and submitted over a couple of days. I can’t thank my approver enough for the diligent review of such tedious minor changes!

There were a few problems…

  • The main one that cropped up was the result of the goimports call above. In some cases, there were multiple files in the package, but the goimports above was only running on a single file, so I saw some unexpected imports for names that were pulled in from a different file in the same package. These were quickly identified and fixed, as they caused test failures!
  • One problem happened twice: a local variable named time shadowed the newly added import of the time package. This was trivially fixable with a local rename. Twice out of 4100 files is OK to do by hand.
  • In a few cases I discovered areas of code that had remained on Go 1.16 (temporarily). These broke very quickly when the tests were executed. I simply reverted these changes and will revisit them in a month or two.

Overall this is a pretty positive experience. I was able to upgrade a large swathe of Google’s code with fairly little fuss. Nobody asked me to do it — and I didn’t need to ask much in the way of permission. The end result is that code inside Google is more similar to code outside.

The main downside is that I should have been working on migrating a metrics pipeline, and instead I got a bit distracted by an interesting problem. Ah well.

Stats: 267 submitted (244 were the LSC described above), 30 reviewed

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s