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.

Sign up for GitHub

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

Jump to bottom

Fixed includes in experimental headers #1805

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Fixed includes in experimental headers #1805

merged 1 commit into from
Jun 13, 2023

Conversation

andriy-kokhan
Copy link
Contributor

No description provided.

andriy-kokhan
andriy-kokhan commented Jun 9, 2023
View reviewed changes
experimental/saiextensions.h
@@ -25,6 +25,7 @@
#ifndef __SAIEXTENSIONS_H_
#define __SAIEXTENSIONS_H_

#include <sai.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing include for SAI_API_MAX used below

andriy-kokhan
andriy-kokhan commented Jun 9, 2023
View reviewed changes
experimental/saiswitchextensions.h
@@ -25,6 +25,7 @@
#ifndef __SAISWITCHEXTENSIONS_H_
#define __SAISWITCHEXTENSIONS_H_

#include <saiswitch.h>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed include for SAI_SWITCH_ATTR_END used below

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 9, 2023

So this is not necessay since usually its used like
#include sai.h
#include experimental.h

@andriy-kokhan
Copy link
Contributor Author

andriy-kokhan commented Jun 9, 2023 •
edited
Loading

So this is not necessay since usually its used like #include sai.h #include experimental.h

@kcudnik
The commands as follow cause errors in logs because of this implicit dependency

cd sonic-sairedis && git submodule update --init
./autogen.sh
dpkg-buildpackage -us -uc -b --target=binary-syncd-vs

But we can easily avoid this implicit dependency which is a bad practice anyway. So, why not?

BTW, "saitypes.h" is also included in "sai.h" but still included in these experimental headers.

@andriy-kokhan
Copy link
Contributor Author

@rlhui , @kcudnik , can we merge this?

@kcudnik kcudnik merged commit c97227c into opencomputeproject:master Jun 13, 2023
JaiOCP pushed a commit to JaiOCP/SAI that referenced this pull request Jun 26, 2023
@andriy-kokhan @JaiOCP
Fixed includes in experimental headers (opencomputeproject#1805)
eb441fe 
Signed-off-by: Andriy Kokhan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kcudnik kcudnik kcudnik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andriy-kokhan @kcudnik