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

core: add os::copy_file function #2090

Closed
wants to merge 1 commit into from
Closed

Conversation

@thomaslee
Copy link
Contributor

@thomaslee thomaslee commented Mar 31, 2012

This adds os::copy_file to libcore as requested in #1983.

I'll submit relevant changes to cargo as a separate pull request.

@ghost ghost assigned catamorphism Mar 31, 2012
@catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Mar 31, 2012

Great, I'll review this.

Loading

@catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Apr 2, 2012

Haven't forgotten this -- the code looks fine, but I added a test case and am having problems on Linux/FreeBSD. (Probably my fault.) Working on it.

Loading

@thomaslee
Copy link
Contributor Author

@thomaslee thomaslee commented Apr 3, 2012

I would've added tests myself, but it wasn't obvious to me where they should be added -- happy to take a look if you've got a working branch up somewhere!

Loading

@catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Apr 3, 2012

It's always fine to add tests under src/test -- run-fail, run-pass, and compile-fail are subdirectories under that one, in which tests get placed depending on their expected behavior. Also, some libraries, including os, have a test module with tests annotated with the #[test] attribute. The tests I added work now (my issue had to do with the working directory being different on different buildbots -- not your fault at all!) and I'll be merging this shortly. Feel free to add more tests, and thanks for the patch!

Loading

@catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Apr 3, 2012

Merged as e9ff495 -- thanks again!

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants