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

rm: rm3.sh now passes #4008

Closed
wants to merge 15 commits into from
Closed

rm: rm3.sh now passes #4008

wants to merge 15 commits into from

Conversation

palaster
Copy link
Contributor

@palaster palaster commented Oct 6, 2022

  • Changed the use of hard-coded prompts messages to use uucore::util_name instead
  • Rewrote prompt_write_protected and prompt_file into a single function prompt_file because I a lot of the code was being reused for both

remove_file(file, options)
} else {
match file.metadata() {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment explaining in which case we are here

remove_file(file, options)
}
}
Err(_e) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a test to cover this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is already a test for it: test_rm::test_rm_symlink_dir() and test_rm::test_rm_invalid_symlink()

Copy link
Contributor

Choose a reason for hiding this comment

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

the code coverage results don't agree ;)

Copy link
Contributor

@sylvestre sylvestre left a comment

Choose a reason for hiding this comment

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

please add a test in the rust test suite

tertsdiepraam and others added 15 commits October 8, 2022 14:54
Generating the tests to run in build.rs created problems for tooling. For example, cargo fmt, was ignoring the test_*.rs files and needed to be passed these files manually to be formatted. Now we simply use the feature mechanism to decide which tests to run.
Correct the error message produced when attempting to copy a directory
into itself with `cp`. Before this commit, the error message was

    $ cp -R d d
    cp: cannot copy a directory, 'd', into itself, 'd'

After this commit, the error message is

    $ cp -R d d
    cp: cannot copy a directory, 'd', into itself, 'd/d'
Move the copy-on-write functions for `cp` to their own module. This
provides a layer of indirection so that the `cp.rs` module need only
use `platform::copy_on_write()`, and the `platform` module is
responsible for providing the appropriate implementation for the
current platform. This commit does not change the behavior of the
code, just its organization.
This commit now allows split to pass split/numeric.sh
@palaster
Copy link
Contributor Author

palaster commented Oct 8, 2022

Broke branch will have to make a new one: new one here

@palaster palaster closed this Oct 8, 2022
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.

6 participants