Skip to content

Bugfix/date arg types #1684

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

cburgos
Copy link
Contributor

@cburgos cburgos commented Jul 30, 2019

Addresses #1662

@cburgos cburgos closed this Jul 30, 2019
@cburgos cburgos reopened this Jul 30, 2019
@ghost
Copy link

ghost commented Jul 30, 2019

Notice that other methods such as setHours also need to allow negative arguments.

@cburgos
Copy link
Contributor Author

cburgos commented Jul 30, 2019

Noted, going through and updating the rest of the methods. Update should be ready shortly.

@alexcrichton
Copy link
Contributor

Thanks! FWIW as-is this is a breaking change so we won't be able to merge this quite just yet, we're still in the process of batching breaking changes. If you'd prefer though perhaps new methods with new names could be added, and then in the next major version release we can delete all the methods and change the existing signatures?

@cburgos
Copy link
Contributor Author

cburgos commented Jul 30, 2019

Added the other updates. @alexcrichton I think holding off and batching these with other breaking changes makes sense. Also worth noting the above commits didn't include the setFullYear and setUTCFullYear as support for optional parameters have not yet been implemented. It might make more sense to include that with #1664 for overall optional param support.

@alexcrichton alexcrichton added the breaking-change Tracking breaking changes for the next major version bump (if ever) label Jul 30, 2019
@alexcrichton
Copy link
Contributor

Ok sounds good to me! I'll tag this as a breaking change and we'll merge this when we get to doing the next round of breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants