search

frequenz-floss / frequenz-repo-config-python

Frequenz repository setup tools and common configuration for Python
https://frequenz-floss.github.io/frequenz-repo-config-python/
MIT License
3 stars 7 forks source link

Test proto files are properly generated in every language we use #238

Open llucax opened 2 months ago

llucax commented 2 months ago

What's needed?

Once we move away the Python generation from API repositories, we'll still need to make sure proto files can be properly generated.

Proposed solution

Use the protoc compiler to build the proto files, ideally for all language we make bindings for, and the most mainstream languages (like go or C++). Generating more languages might surface issues not present in every language, and we want to ideally stay as compatible as possible with all.

Additional context

One example is detection of unused imports, python and rust generation doesn't detect this, but go generation does.

Please see this PR adding an extra CI step to do this in the microgrid API:

tiyash-basu-frequenz commented 2 months ago

One example is detection of unused imports, python and rust generation doesn't detect this, but go generation does.

I noticed that unused imports are flagged even when using just --python_out or --cpp_out. The thing is that these are just warnings, and not errors. The safest first approach might be to just add --fatal_warnings to protoc calls, so that the protoc calls fail on warnings.

llucax commented 2 months ago

Ah, interesting! We can update repo-config to do this now then.

llucax commented 2 months ago

Moving to v0.10.0.

tiyash-basu-frequenz commented 2 months ago

Ah, interesting! We can update repo-config to do this now then.

Very likely, yes.