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

free: Implement almost all functions of the free commands #35

Merged
merged 16 commits into from
Mar 22, 2024
Merged

free: Implement almost all functions of the free commands #35

merged 16 commits into from
Mar 22, 2024

Conversation

Krysztal112233
Copy link
Collaborator

@Krysztal112233 Krysztal112233 commented Mar 21, 2024

Aims

This pull request aims to implement almost all function about free command.

And also fix wron arguments and flags.

TODO

@sylvestre
Copy link
Contributor

great start but
please don't implement everything into a single PR

@Krysztal112233 Krysztal112233 marked this pull request as ready for review March 21, 2024 15:26
@Krysztal112233
Copy link
Collaborator Author

great start but please don't implement everything into a single PR

Okay, next I'll split it into multiple PR.

So what should this PR do next?

src/uu/free/src/free.rs Outdated Show resolved Hide resolved
src/uu/free/src/free.rs Outdated Show resolved Hide resolved
src/uu/free/src/free.rs Outdated Show resolved Hide resolved
src/uu/free/src/free.rs Outdated Show resolved Hide resolved
src/uu/free/src/units.rs Outdated Show resolved Hide resolved
@Krysztal112233
Copy link
Collaborator Author

I have a question about using if instead of match:

Sometimes match looks cleaner and more direct, so why not use match? Because match won't be optimized by branch prediction?

@sylvestre
Copy link
Contributor

sylvestre commented Mar 22, 2024

Sometimes match looks cleaner and more direct, so why not use match?

Because match is used when we have several diverse values.
For such operations, the de facto standard is to use if instead.
it is also for consistency with coreutils

Because match won't be optimized by branch prediction?

I hope that, between Rust and LLVM, it will generate the same code.

@sylvestre
Copy link
Contributor

sylvestre commented Mar 22, 2024

https://godbolt.org/z/Ec3aedhWG confirms that is generate the same code

@Krysztal112233
Copy link
Collaborator Author

https://godbolt.org/z/Ec3aedhWG confirms that is generate the same code

I got it

@Krysztal112233
Copy link
Collaborator Author

completes almost all the most often flags for the free command now

@sylvestre sylvestre merged commit 9b78ffa into uutils:main Mar 22, 2024
3 of 13 checks passed
@sylvestre
Copy link
Contributor

Sorry, i only realize thi now. Could you please add tests in
https://github.com/uutils/procps/blob/main/tests/by-util/test_free.rs
thanks

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.

2 participants