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

feat(24.04): Add nodejs slice #121

Merged
merged 7 commits into from
Feb 7, 2024
Merged

feat(24.04): Add nodejs slice #121

merged 7 commits into from
Feb 7, 2024

Conversation

linostar
Copy link

@linostar linostar commented Feb 5, 2024

Adding nodejs slice to 24.04.

cjdcordeiro
cjdcordeiro requested changes Feb 6, 2024
View reviewed changes
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

You seem to be missing libc-ares2

@linostar
Copy link
Author

linostar commented Feb 6, 2024

You seem to be missing libc-ares2

Added now.

@linostar linostar requested a review from cjdcordeiro February 6, 2024 20:10
rebornplusplus
rebornplusplus reviewed Feb 7, 2024
View reviewed changes
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looks nice. I left a few comments. Let me know what you think of them.

slices/nodejs.yaml Show resolved Hide resolved
slices/libnode108.yaml Show resolved Hide resolved
slices/libnode108.yaml Outdated Show resolved Hide resolved
slices/node-cjs-module-lexer.yaml Show resolved Hide resolved
rebornplusplus
rebornplusplus approved these changes Feb 7, 2024
View reviewed changes
Copy link
Member

@rebornplusplus rebornplusplus left a comment

Choose a reason for hiding this comment

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

Looks good to me. 👍 Just left two nitpick comments about sorting and adding comments in nodejs.yaml.

slices/nodejs.yaml Outdated Show resolved Hide resolved
cjdcordeiro
cjdcordeiro approved these changes Feb 7, 2024
View reviewed changes
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

tyvm. seems to work well and I agree with the decision for libatomic.

@cjdcordeiro cjdcordeiro merged commit afadfc2 into canonical:ubuntu-24.04 Feb 7, 2024
@linostar linostar deleted the feat/add-nodejs-to-24.04 branch February 7, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rebornplusplus rebornplusplus rebornplusplus approved these changes

@cjdcordeiro cjdcordeiro cjdcordeiro approved these changes

Assignees

@cjdcordeiro cjdcordeiro

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@linostar @cjdcordeiro @rebornplusplus