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

std.format: clean imports #2666

Merged
merged 1 commit into from Nov 10, 2014
Merged

std.format: clean imports #2666

merged 1 commit into from Nov 10, 2014

Conversation

9il
Copy link
Member

@9il 9il commented Nov 8, 2014

}
version(unittest) {
import core.vararg;
import std.conv, std.exception, std.range, std.traits, std.typetuple;
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to also move std.conv into local imports? The fewer module-global imports, the better IMO, since it will make it much easier to split up Phobos modules into smaller, independent pieces.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. fixed

@joakim-noah
Copy link
Contributor

btw, have you thought about writing an automated tool to make cleaning up imports easier? I have suggested the feature for dfix, maybe it's something you'd want to implement. It'd make cleaning up phobos much easier and be more widely useful for others too.

@9il
Copy link
Member Author

9il commented Nov 8, 2014

Good idea! But phobos needs review. I have found 2 bugs.

remove new private stuff

add local import

add local import

remove global std.con import
@9il
Copy link
Member Author

9il commented Nov 8, 2014

rebased

nested = to!(typeof(nested))(trailing[i + 1 .. k - 1]);
sep = to!(typeof(nested))(trailing[k + 1 .. j - 1]);
nested = trailing[i + 1 .. k - 1];
sep = trailing[k + 1 .. j - 1];
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason is to don't import std.conv: this to! do nothing for const(Char)[].

Copy link
Member Author

Choose a reason for hiding this comment

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

const(Char)[] a;
...
assert(to!(typeof(a))a is a);

Copy link
Member

Choose a reason for hiding this comment

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

Got it, thanks!

@quickfur
Copy link
Member

LGTM

@quickfur
Copy link
Member

Auto-merge toggled on

@9il
Copy link
Member Author

9il commented Nov 10, 2014

Thanks!

quickfur pushed a commit that referenced this pull request Nov 10, 2014
std.format: clean imports
@quickfur quickfur merged commit 2ec073a into dlang:master Nov 10, 2014
@9il 9il deleted the format branch November 10, 2014 20:15
@9il 9il restored the format branch November 10, 2014 23:02
@9il
Copy link
Member Author

9il commented Nov 10, 2014

My bad. Please merge #2683.

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