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

Improve std.conv.to (2) #119

Merged
merged 8 commits into from
Jul 13, 2011
Merged

Improve std.conv.to (2) #119

merged 8 commits into from
Jul 13, 2011

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Jun 23, 2011

The main commit is b0c6d7e8, and others are same as pull #118.

Changes:
3. Support conversion feature using construction like new T(s) with class types.

@jmdavis
Copy link
Member

jmdavis commented Jun 26, 2011

In the future, can you either do these multi-part pull requests as one request or wait until one is merged in before doing the others? As it is, they end up sharing a lot of commits which makes reviewing them harder.

@9rnsr
Copy link
Contributor Author

9rnsr commented Jun 26, 2011

I have thought that amount of commits was a bit problem.
At the next opportunity, I want to improve the way dividing commits as pull requests.

@jmdavis
Copy link
Member

jmdavis commented Jun 26, 2011

Oh, it is a lot of commits to have in one pull request. It's just that splitting it like this doesn't really help all that much, since each successive pull request has all of the comits that the previous one had, so the last pull request still has all of them in it, and when you look at each pull request, you then have to figure out which commits you've already looked at. Really, I would say that the first two should have been done as one, and since the 3rd one rearranges everything so much, it probably should have been done after the first two were merged in. Then none of the changes from the first 2 pull requests would be listed as part of it (since they'd already be in the master branch of the main repository).

Figuring out how best to separate large changes like this can be a challenge (e.g. after having done it, I came to the conclusion that my pull request for std.string likely should have been split up more, with me waiting for each of the previous ones to be merged in before doing the next one), but the way that you did it didn't really separate them, because they're not unrelated, and each successive one has all of the commits of the previous one.

9rnsr added 8 commits July 7, 2011 08:18
…straint of S -> constraint of T) for code readability.
…ible

The two templates has same purpose, but bit different behaviors.
implicitlyConverts(S, T) has following semantics:
   is(S : T) and allow initializer conversion (T t = s; -> T t = T(s);)
   Including initializer conversion is inconvenience for my to!() improvement in after commits, so replace it.
1. Remove conversion feature using member template function to!T() of user type.
2-1. Support conversion feature using opCast!T() with aggregate types.
2-2. Support conversion feature using constructor like T(s) with struct types.
@9rnsr
Copy link
Contributor Author

9rnsr commented Jul 6, 2011

Rebase only.

jmdavis added a commit that referenced this pull request Jul 13, 2011
@jmdavis jmdavis merged commit 6478323 into dlang:master Jul 13, 2011
kuettler pushed a commit to kuettler/phobos that referenced this pull request Feb 6, 2018
Move rdmd.1 to a man directory to ease packaging
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