Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
llucax opened this issue Apr 12, 2024 · 4 comments
Open

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

llucax opened this issue Apr 12, 2024 · 4 comments
Assignees
Labels
part:ci Affects the GitHub workflow and other parts for running CI part:only-api Affects the configuration of a api repo part:template Affects the cookiecutter template files type:enhancement New feature or enhancement visitble to users
Milestone

Comments

@llucax
Copy link
Contributor

llucax commented Apr 12, 2024

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:

@llucax llucax added part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed type:enhancement New feature or enhancement visitble to users part:ci Affects the GitHub workflow and other parts for running CI part:only-api Affects the configuration of a api repo part:template Affects the cookiecutter template files and removed part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed labels Apr 12, 2024
@llucax llucax added this to the Untriaged milestone Apr 12, 2024
@tiyash-basu-frequenz
Copy link
Contributor

tiyash-basu-frequenz commented Apr 12, 2024

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
Copy link
Contributor Author

llucax commented Apr 16, 2024

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

@llucax llucax modified the milestones: Untriaged, v0.10.0 Apr 16, 2024
@llucax
Copy link
Contributor Author

llucax commented Apr 16, 2024

Moving to v0.10.0.

@llucax llucax self-assigned this Apr 16, 2024
@tiyash-basu-frequenz
Copy link
Contributor

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

Very likely, yes.

@llucax llucax modified the milestones: v0.10.0, v0.11.0 Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:ci Affects the GitHub workflow and other parts for running CI part:only-api Affects the configuration of a api repo part:template Affects the cookiecutter template files type:enhancement New feature or enhancement visitble to users
Projects
None yet
Development

No branches or pull requests

2 participants