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

[feature] Implement a clone method for durations, fixes #4078 #4080

Closed
wants to merge 3 commits into
base: develop
from

Conversation

Projects
None yet
3 participants
@ashsearle
Contributor

ashsearle commented Jul 21, 2017

No description provided.

var durationB = durationA.clone();
durationA.add(5, 'milliseconds');
assert.equal(durationB.milliseconds(), 5, 'Calling duration.clone() on a duration will create a clone');
assert.equal(durationA.milliseconds(), 10, 'Calling duration.clone() on a duration will create a clone');

This comment has been minimized.

@maggiepint

maggiepint Jul 25, 2017

Member

It would make more sense to test that durationA.milliseconds() does not equal durationB.milliseconds(). That would test that the object had in fact been cloned more explicitly

This comment has been minimized.

@ashsearle

ashsearle Jul 26, 2017

Contributor

I agree - I've updated code, along with the original unit-tests I copied this logic from.

@maggiepint maggiepint referenced this pull request Jul 25, 2017

Closed

Add Duration.clone() #4078

ashsearle added some commits Jul 26, 2017

@ichernev ichernev changed the title from Fix #4078: Implement a clone method for durations to [feature] Implement a clone method for durations, fixes #4078 Aug 3, 2017

@ichernev

This comment has been minimized.

Contributor

ichernev commented Aug 7, 2017

Merged in 2847a99

@ichernev ichernev closed this Aug 7, 2017

ichernev added a commit that referenced this pull request Aug 7, 2017

Merge pull request #4080 from ashsearle:fix/4078
[feature] Implement a clone method for durations, fixes #4078

@ashsearle ashsearle deleted the ashsearle:fix/4078 branch Aug 8, 2017

ichernev added a commit that referenced this pull request Oct 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment