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

Commit generated DASH SAI APIs into source control #361

Open
mukeshmv opened this issue Apr 27, 2023 · 2 comments
Open

Commit generated DASH SAI APIs into source control #361

mukeshmv opened this issue Apr 27, 2023 · 2 comments

Comments

@mukeshmv
Copy link
Collaborator

mukeshmv commented Apr 27, 2023

DASH SAI APIs are generated from bmv2 P4 code. Any PR with P4 changes can change the generated DASH SAI APIs. However it is not easy to review these changes unless we checkout the PR in our local workspaces, build it and compare it with a copy of the previous DASH SAI APIs.

It might make it easier for a lot more people to review it if the DASH SAI API changes also showed up in the diff in the PR itself. One way of doing this would be to commit the generated APIs also to the DASH git repo. If needed this could be kept separate from the SAI submodule by placing it say under dash-pipeline/generated/ with the sole purpose of viewing the current snapshot / for review purposes so as to not disturb any of the existing workflows. This requires discipline from the developers to keep updating the folder for every PR.

This can also work as a quick reference for whoever wants to look at the current generated DASH SAI APIs since the OCP SAI repo may lag behind the latest features added in the DASH repo.

@chrispsommers
Copy link
Collaborator

@mukeshmv Excellent suggestion, and thank you for posting this. I thought about a way to ensure a developer checks in
generated SAI (only if it changed): we could make a new CI compare-sai script which compares the committed, generated SAI (dash-pipeline/generated) with SAI generated during the CI run of make sai P4 (under sai/experimental) and if they disagree, it fails (or something like this). Then the PR will not get accepted. I think this would be fairly straightforward.

@mukeshmv
Copy link
Collaborator Author

Thanks @chrispsommers. That sounds perfect !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants