Skip to content

add option to ignore out of line modules #719

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

Merged
merged 1 commit into from
Dec 23, 2015

Conversation

matklad
Copy link
Member

@matklad matklad commented Dec 23, 2015

Fixes #400.

I'm not sure how should I add a test for the option :)

@nrc
Copy link
Member

nrc commented Dec 23, 2015

To test an option you add a comment like this to your test.

@nrc
Copy link
Member

nrc commented Dec 23, 2015

I guess the tricky thing with tests is that if there is a sub-module that we expect not to be formatted, then it will appear in the target dir as unformatted, however, the test infra when finding the file as a top-level file will want to reformat it, so there will be an error.

I think the solution is to add a don't test directive to the testing infra which can be added to a file and will skip testing for that file as a top-level file, but not as a sub-module. Although tbh, I don't think our testing infra is module aware in any case, so maybe we can't test this at all.

@nrc
Copy link
Member

nrc commented Dec 23, 2015

OK, just going to merge, who needs tests any way? If you think of a way to test this or want to modify the testing infra, that would be awesome, but I don't want to block this on that.

nrc added a commit that referenced this pull request Dec 23, 2015
add option to ignore out of line modules
@nrc nrc merged commit 9107fac into rust-lang:master Dec 23, 2015
@nrc
Copy link
Member

nrc commented Dec 23, 2015

Thanks for the PR!

matklad added a commit to matklad/rustfmt that referenced this pull request Dec 23, 2015
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