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

Added package to compile and link yaml parser in C++ #20698

Merged
merged 5 commits into from
Nov 18, 2024

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Nov 5, 2024

What I did:
Added package to compile and link yaml file parser in C++

Why I did:

To compile and link changes done in PR: sonic-net/sonic-swss#3353

How I verify:
Manual Verification

@abdosi
Copy link
Contributor Author

abdosi commented Nov 9, 2024

@saiarcot895 : can you please help in review this.

@@ -16,6 +16,7 @@ RUN apt-get update && \
libc-ares2 \
iproute2 \
logrotate \
libyaml-cpp-dev \
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming you don't actually need the development headers at runtime, there shouldn't be any changes needed in this file. Dependency resolution should be able to install the runtime library automatically.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@saiarcot895 : not sure if i completely follow this comment. Without this change I was getting run-time error when fpmsyncd was running in bgp docker. PR for reference: sonic-net/sonic-swss#3353

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @saiarcot895 , i have removed the change. Yes, as part of the build of docker-swss-layer-bookworm.gz all swss dependency are resolved and we don't need to explicit install on in frr docker.

@abdosi
Copy link
Contributor Author

abdosi commented Nov 13, 2024

@saiarcot895 : can we get signoff on this,

@abdosi
Copy link
Contributor Author

abdosi commented Nov 15, 2024

/azpw ms_conflict

2 similar comments
@abdosi
Copy link
Contributor Author

abdosi commented Nov 15, 2024

/azpw ms_conflict

@abdosi
Copy link
Contributor Author

abdosi commented Nov 15, 2024

/azpw ms_conflict

@abdosi
Copy link
Contributor Author

abdosi commented Nov 15, 2024

'/azpw ms_conflict'

@abdosi
Copy link
Contributor Author

abdosi commented Nov 16, 2024

/azpw ms_conflict

1 similar comment
@abdosi
Copy link
Contributor Author

abdosi commented Nov 17, 2024

/azpw ms_conflict

@abdosi
Copy link
Contributor Author

abdosi commented Nov 18, 2024

@rlhui : can you help merge this.

@rlhui rlhui merged commit 8b5166d into sonic-net:master Nov 18, 2024
12 checks passed
@abdosi
Copy link
Contributor Author

abdosi commented Nov 18, 2024

@yejianquan for 202405 branch

@yejianquan
Copy link
Contributor

Having conflict @abdosi , please create a seperate PR to 202405 and add to wishlist

@abdosi
Copy link
Contributor Author

abdosi commented Nov 19, 2024

Having conflict @abdosi , please create a seperate PR to 202405 and add to wishlist

@yejianquan : PR for 202405: #20856

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants