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

Switch to jansson sm Remove YAJL and Jansson #146

Merged
merged 6 commits into from
Nov 27, 2024

Conversation

xw19
Copy link

@xw19 xw19 commented Nov 26, 2024

Remove yajl and jansson as gitsubmodule

Signed-off-by: Sourav Moitra <[email protected]>
Signed-off-by: Sourav Moitra <[email protected]>
Signed-off-by: Sourav Moitra <[email protected]>
@xw19 xw19 changed the title Switch to jansson sm Switch to jansson sm Remove YAJL and Jansson Nov 26, 2024
@xw19
Copy link
Author

xw19 commented Nov 26, 2024

@saschagrunert PTAL

Copy link

@themoriarti themoriarti left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

Just 2 nits, otherwise LGTM

.github/workflows/test.yaml Outdated Show resolved Hide resolved
Containerfile Outdated Show resolved Hide resolved
Signed-off-by: Sourav Moitra <[email protected]>
@xw19
Copy link
Author

xw19 commented Nov 27, 2024

@saschagrunert PTAL

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

The dev package should also provide the lib, right?

install: |
apt-get update -q -y
apt-get install -q -y python3 automake libtool autotools-dev git make cmake pkg-config gcc wget xz-utils
apt-get install -q -y python3 automake libtool autotools-dev git make cmake pkg-config gcc wget xz-utils libjansson4 libjansson-dev
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
apt-get install -q -y python3 automake libtool autotools-dev git make cmake pkg-config gcc wget xz-utils libjansson4 libjansson-dev
apt-get install -q -y python3 automake libtool autotools-dev git make cmake pkg-config gcc wget xz-utils libjansson-dev

Copy link
Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

Choose a reason for hiding this comment

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

I was wrong, it broke the CI

Containerfile Outdated Show resolved Hide resolved
Signed-off-by: Sourav Moitra <[email protected]>
@saschagrunert saschagrunert merged commit 19192a0 into containers:switch-to-jansson Nov 27, 2024
6 checks passed
@saschagrunert
Copy link
Member

Refers to #138

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.

3 participants