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

Add gRIBI support for OC AFT model 2.7.0 #93

Merged
merged 7 commits into from
Oct 17, 2024
Merged

Add gRIBI support for OC AFT model 2.7.0 #93

merged 7 commits into from
Oct 17, 2024

Conversation

dplore
Copy link
Member

@dplore dplore commented Sep 25, 2024

Brings gRIBI up to date from OpenConfig AFT model v2.1.0 to v2.7.0

Highlighted updates which impact gRIBI

Other AFT model revisions did not impact gRIBI as they are related to leaves or attributes not directly related or accessible via the gRIBI RPCs.

@dplore dplore changed the title Add OC AFT model 2.7.0 Add gRIBI support for OC AFT model 2.7.0 Sep 25, 2024
Copy link

@vishnureddybadveli vishnureddybadveli left a comment

Choose a reason for hiding this comment

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

Taking a first pass review

Copy link
Contributor

@nandanarista nandanarista left a comment

Choose a reason for hiding this comment

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

lgtm

@dplore
Copy link
Member Author

dplore commented Oct 11, 2024

I need to refresh this with minor updates since openconfig/public#1153 was merged. I'll do this today

@dplore
Copy link
Member Author

dplore commented Oct 16, 2024

@robshakir This is ready for review

Copy link
Contributor

@robshakir robshakir left a comment

Choose a reason for hiding this comment

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

Part way through reviewing -- aeroplane WiFi and GitHub are not super-compatible.

scripts/README.md Outdated Show resolved Hide resolved
scripts/README.md Show resolved Hide resolved
scripts/README.md Outdated Show resolved Hide resolved
scripts/README.md Outdated Show resolved Hide resolved
scripts/README.md Outdated Show resolved Hide resolved
scripts/update-schema.sh Show resolved Hide resolved
Copy link
Contributor

@robshakir robshakir left a comment

Choose a reason for hiding this comment

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

Thanks for the changes here -- generally, LGTM. Few small nits :-)

v1/proto/gribi_aft/gribi_aft.proto Outdated Show resolved Hide resolved
v1/proto/gribi_aft/gribi_aft.proto Outdated Show resolved Hide resolved
v1/yang/aft/openconfig-aft-network-instance.yang Outdated Show resolved Hide resolved
Copy link
Member Author

@dplore dplore left a comment

Choose a reason for hiding this comment

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

Deviated out vxlan
Revised update-schema.sh
Removed aft network-instance yang dep
Ready for review

scripts/README.md Outdated Show resolved Hide resolved
scripts/README.md Outdated Show resolved Hide resolved
scripts/README.md Show resolved Hide resolved
scripts/README.md Outdated Show resolved Hide resolved
scripts/README.md Outdated Show resolved Hide resolved
scripts/update-schema.sh Show resolved Hide resolved
v1/proto/gribi_aft/gribi_aft.proto Outdated Show resolved Hide resolved
v1/yang/aft/openconfig-aft-network-instance.yang Outdated Show resolved Hide resolved
@dplore dplore requested a review from robshakir October 17, 2024 03:26
@@ -38,7 +39,8 @@ if [ -z $SRCDIR ]; then
SRC_DIR=`runreadlink -m ${THIS_DIR}/..`
fi

proto_generator \
echo ${SRC_DIR}
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove echo?

Copy link
Contributor

@robshakir robshakir left a comment

Choose a reason for hiding this comment

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

Please remove the echo then LGTM. Thanks.

@dplore
Copy link
Member Author

dplore commented Oct 17, 2024

Please remove the echo then LGTM. Thanks.

done!

@dplore dplore merged commit 024356c into master Oct 17, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

4 participants