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

Change newlib dependency with musl #7

Closed
wants to merge 1 commit into from
Closed

Change newlib dependency with musl #7

wants to merge 1 commit into from

Conversation

JADarius
Copy link
Contributor

@JADarius JADarius commented Jul 15, 2023 •
edited
Loading

This PR changes the C library dependency from newlib to musl.

eduardvintila
eduardvintila requested changes Jul 15, 2023
View reviewed changes
Copy link
Member

@eduardvintila eduardvintila left a comment

Choose a reason for hiding this comment

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

This does the job, thank you. You should include a more descriptive message in the commit description, following the contribution guidelines

@JADarius JADarius changed the title Modified patch Modify patch Jul 15, 2023
@JADarius JADarius changed the title Modify patch Change newlib dependency with musl Jul 15, 2023
eduardvintila
eduardvintila requested changes Jul 15, 2023
View reviewed changes
Copy link
Member

@eduardvintila eduardvintila 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 - just update the commit message to use "Change" instead of "change" and I'll be ready to approve 👍🏻

@eduardvintila eduardvintila self-requested a review July 15, 2023 15:15
eduardvintila
eduardvintila approved these changes Jul 15, 2023
View reviewed changes
Copy link
Member

@eduardvintila eduardvintila left a comment

Choose a reason for hiding this comment

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

All good, thank you!

Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com

StefanJum
StefanJum requested changes Jul 17, 2023
View reviewed changes
Copy link
Member

@StefanJum StefanJum left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you @JADarius.
The changes to patches/ will be overriten by #8, so you can remove them.
Everything looks fine besides that.

@JADarius JADarius requested a review from StefanJum July 18, 2023 06:03
razvand
razvand requested changes Jul 18, 2023
View reviewed changes
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

@JADarius, squash the two commits in a single commit. The commit should only be related to the Musl dependency. Use Config.uk as commit message prefix. No need to use lib-lua, it's implicit.

@razvand razvand mentioned this pull request Jul 18, 2023
Closed
@JADarius
Config.uk: Change newlib dependency to musl
07f32a0 
Signed-off-by: Darius-Andrei Jipa <jipad16@yahoo.com>
@JADarius JADarius requested a review from razvand July 22, 2023 10:48
StefanJum
StefanJum approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Thanks @JADarius.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

razvand
razvand approved these changes Jul 25, 2023
View reviewed changes
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Approved-by: Razvan Deaconescu razvand@unikraft.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@razvand razvand razvand approved these changes

@eduardvintila eduardvintila eduardvintila approved these changes

@StefanJum StefanJum StefanJum approved these changes

@RaduNichita RaduNichita Awaiting requested review from RaduNichita

Assignees

@razvand razvand

Labels
ci/merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@JADarius @razvand @eduardvintila @StefanJum @unikraft-bot