Handling auto-generated protobuf files

[dbwiddis]

While reviewing our code coverage I noticed the majority of our uncovered lines (57 of 102) are in the protobuf subdirectory.

Initial question: should we exclude these files from coverage (I think so).

Follow-up question: Where else should (or shouldn't) we exclude the auto-generated files?

• Should they be excluded from being committed (.gitignore) so we don't need to worry about it?
  • That puts a requirement on the user to generate them, which we shouldn't require unless it's built into how they run it (e.g., poetry) so I'm happy with them being in the repo. Plus we don't change them often.
• I excluded from formatting because they said "do not edit" but it doesn't hurt to format them.
• It looks like they aren't excluded from license headers. I briefly looked into this and generated files generally are considered an "object" with the same license as the source (much like compiled code) so it doesn't hurt to add the license here.
  • But since we're "modifying" the file to add the license header, perhaps we should just go ahead and modify them with formatting scripts.

TLDR:

• I think we should exclude proto files from code coverage
• I think we should remove the exclusion from formatting tools

[dblock]

I think the question is whether these files are part of source code, or part of generated files at build time. Your tl;dr says it's the latter, which means we should build them as part of poetry install or something like that.

[dbwiddis]

An added complication of build time, however, is that protoc generates proto3-incompatible import statments, see protocolbuffers/protobuf#1491. I eventually figured out that 2to3 is one compatible workaround.

Given how little (if ever) these files are going to change, I'm fine with pre-building them. It's more of a question of how much we should touch them with additional automation.

[dblock]

IMO, since these are generated files we should make sure they have a this is generated header and otherwise not touch them.

coverage-wise though they should be used, otherwise they aren’t useful files and shouldn’t exist

[dblock]

In #54 (comment) we suggest removing the lines of generating proto files from CI and documenting how to update them.