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

kill: return 1 and gnu style stderr in case of no pid #6225

Merged
merged 2 commits into from
Apr 14, 2024

Conversation

jadijadi
Copy link
Contributor

including a new test for "no pid provided" use case.

closes #6221

Copy link

GNU testsuite comparison:

GNU test failed: tests/cp/same-file. tests/cp/same-file is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)
GNU test failed: tests/cp/cross-dev-symlink. tests/cp/cross-dev-symlink is passing on 'main'. Maybe you have to rebase?

@BenWiederhake BenWiederhake self-requested a review April 14, 2024 02:01
Copy link
Collaborator

@BenWiederhake BenWiederhake left a comment

Choose a reason for hiding this comment

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

Does exactly what is says on the tin. Thank you!

Can I ask you to change the excessive indentation?

(Also, sorry about the "review self-request", I fully expected it to be an error message, because self-requesting makes to sense to me, and I was curious how github handled that edge case.)

src/uu/kill/src/kill.rs Outdated Show resolved Hide resolved
Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
Copy link

GNU testsuite comparison:

GNU test failed: tests/cp/same-file. tests/cp/same-file is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/cross-dev-symlink. tests/cp/cross-dev-symlink is passing on 'main'. Maybe you have to rebase?

@jadijadi
Copy link
Contributor Author

Does exactly what is says on the tin. Thank you!
Can I ask you to change the excessive indentation?

Thank you @BenWiederhake . Also applied your suggested change.

@cakebaker cakebaker merged commit 9b4a787 into uutils:main Apr 14, 2024
62 of 64 checks passed
@cakebaker
Copy link
Contributor

Thanks!

sylvestre pushed a commit to sylvestre/coreutils that referenced this pull request Apr 21, 2024
* kill: return 1 and gnu style stderr in case of no pid

closes uutils#6221

* Update src/uu/kill/src/kill.rs

Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>

---------

Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
sylvestre pushed a commit to sylvestre/coreutils that referenced this pull request Apr 21, 2024
* kill: return 1 and gnu style stderr in case of no pid

closes uutils#6221

* Update src/uu/kill/src/kill.rs

Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>

---------

Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
sylvestre pushed a commit to sylvestre/coreutils that referenced this pull request Apr 21, 2024
* kill: return 1 and gnu style stderr in case of no pid

closes uutils#6221

* Update src/uu/kill/src/kill.rs

Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>

---------

Co-authored-by: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
@sylvestre
Copy link
Contributor

@jadijadi thanks for sharing your experience here :) https://www.youtube.com/watch?v=wzWNkNPHJx0

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.

kill: should fail if no PID is specified
4 participants