-
Notifications
You must be signed in to change notification settings - Fork 484
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
Remove use of vendor specific macros #1975
base: master
Are you sure you want to change the base?
Conversation
pleasse add authors of code that is being removed to disscuss whether this is safe |
SAI should not care about which vendor is loaded, so remove the compile time requirement to specify which vendor will be. Signed-off-by: Christian Svensson <[email protected]>
83ced79
to
84ed41d
Compare
Sure, looking at older PRs I guess this is @chrispsommers and @richardyu-ms. I hope this ping is sufficient to ask for a review :-) |
did you make test with your changes in different platforms, meanwhile did you run those change in build-image repo? |
I have tested this successfully on Broadcom and Innovium SAI. |
Cause in your changes, there are some changes to remove the build parameters across many platforms
I think it is better to build the image with your code on those platforms at least, it can make sure the sai component will not impact the SONiC image build. |
@richardyu-ms I do not have any possibility of testing all those platforms. It seems to me that this code is dead and unmaintained, and I would be surprised if anyone with Barefoot or Cavium runs this code. If you are certain you require testing on these platforms to remove this code, I will be forced to give up on this PR.
Is the |
SAI should not care about which vendor is loaded, so remove the compile time requirement to specify which vendor will be.
Proposal from discussions in #1972