Skip to content

/ rust-url

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

Update to rust with opt-in Copy #57

Merged
merged 1 commit into from Dec 10, 2014
Merged

Update to rust with opt-in Copy #57

merged 1 commit into from Dec 10, 2014

Conversation

@ben0x539
Copy link
Contributor

ben0x539 commented Dec 10, 2014

... by adding #[deriving(Copy)] everywhere.
@SimonSapin
Copy link
Member

SimonSapin commented Dec 10, 2014

What’s the motivation for the second commit?
@ben0x539
Copy link
Contributor Author

ben0x539 commented Dec 10, 2014

None on my side, really. Just wanted to be consistent with what was copyable before the change while I was adding deriving bits anyway. Want me to drop it?
@SimonSapin
Copy link
Member

SimonSapin commented Dec 10, 2014

Yes, I’d prefer to be conservative in the promises made (which was the point of the language change of making Copy opt-in) and add more later if needed.
@ben0x539
Update to rust with opt-in Copy
Loading status checks…
1d37cd9 
by adding explicit `Copy` impls for the exported types `SchemeType` and
`EncodeSet`, and also to `EncodingOverride`. This fixes the build for
rustc bc486dc23 2014-12-10.
@ben0x539
Copy link
Contributor Author

ben0x539 commented Dec 10, 2014

Now it's just the three types that need to be Copy for the build to not be broken. We could also be more conservative and only make them Clone and change the code using them instead, but I feel like that decision would be out of scope of my drive-by fixing-my-own-build pull request here.
@SimonSapin
Copy link
Member

SimonSapin commented Dec 10, 2014

Looks good, thanks!
SimonSapin added a commit that referenced this pull request Dec 10, 2014
@SimonSapin
Merge pull request #57 from ben0x539/copy-opt-in
Loading status checks…
c54ef93 
Update to rust with opt-in Copy
@SimonSapin SimonSapin merged commit c54ef93 into servo:master Dec 10, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@SimonSapin SimonSapin mentioned this pull request Dec 12, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@ben0x539 @SimonSapin
You can’t perform that action at this time.