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

README: Update description of Musl support #49

Closed
wants to merge 1 commit into from
Closed

README: Update description of Musl support #49

wants to merge 1 commit into from

Conversation

razvand
Copy link
Contributor

@razvand razvand commented May 11, 2023

The contents of the README.md file are not up-to-date. Replace them with a short description of Musl, and point out that Musl is the default Unikraft libc.

@razvand razvand added documentation Improvements or additions to documentation enhancement New feature or request labels May 11, 2023
@razvand razvand added this to the v0.13.0 (Atlas) milestone May 11, 2023
kubanrob
kubanrob suggested changes May 11, 2023
View reviewed changes
Copy link
Contributor

@kubanrob kubanrob 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, however I am not sure if the information about lib-newlib is correct.

README.md Outdated Show resolved Hide resolved
@razvand razvand requested a review from kubanrob May 11, 2023 14:42
@razvand
Copy link
Contributor Author

razvand commented May 11, 2023

Looks good, however I am not sure if the information about lib-newlib is correct.

Thanks, @kubanrob. I updated the README addressing your comments.

mariasfiraiala
mariasfiraiala suggested changes May 11, 2023
View reviewed changes
Copy link
Contributor

@mariasfiraiala mariasfiraiala left a comment

Choose a reason for hiding this comment

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

Small comment on my part (you'll find the suggestion below). Also, I wonder whether it's worth mentioning the clang - compiler-rt - __muldc3 issue. Even though not many people use clang, it's good for documentation purposes

README.md Outdated Show resolved Hide resolved
@razvand
Copy link
Contributor Author

razvand commented May 12, 2023

Small comment on my part (you'll find the suggestion below). Also, I wonder whether it's worth mentioning the clang - compiler-rt - __muldc3 issue. Even though not many people use clang, it's good for documentation purposes

@mariasfiraiala , done.

mariasfiraiala
mariasfiraiala approved these changes May 12, 2023
View reviewed changes
Copy link
Contributor

@mariasfiraiala mariasfiraiala left a comment

Choose a reason for hiding this comment

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

Looks ok, thanks @razvand!

Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com

eduardvintila
eduardvintila requested changes May 12, 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.

Hey, @razvand. Looks good, thanks for this. There is only one sentence which I would have it rephrased.

README.md Outdated Show resolved Hide resolved
@razvand
README: Update description of Musl support
3713cae 
The contents of the README.md file are not up-to-date. Replace them with
a short description of Musl, and point out that Musl is the default
Unikraft libc.

Signed-off-by: Razvan Deaconescu <razvand@unikraft.io>
@kubanrob
Copy link
Contributor

Thank you @razvand!

Reviewed-by: Robert Kuban robert.kuban@opensynergy.com

eduardvintila
eduardvintila approved these changes May 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.

Thanks, @razvand!

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

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

@kubanrob kubanrob kubanrob approved these changes

@eduardvintila eduardvintila eduardvintila approved these changes

@mariasfiraiala mariasfiraiala mariasfiraiala approved these changes

Assignees

@eduardvintila eduardvintila

Labels
ci/merged documentation Improvements or additions to documentation enhancement New feature or request
Projects
Unikraft Core Roadmap
Status: ✅ Done!
Milestone
v0.13.0 (Atlas)
Development

Successfully merging this pull request may close these issues.
5 participants
@razvand @kubanrob @eduardvintila @mariasfiraiala @unikraft-bot