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

Undeprecate options.date argument to timestamp #1826

Merged
merged 3 commits into from Apr 24, 2018
Merged

Undeprecate options.date argument to timestamp #1826

merged 3 commits into from Apr 24, 2018

Conversation

mborst
Copy link
Contributor

@mborst mborst commented Apr 24, 2018

Introduction

options.date has been deprecated in #1798. This PR reverts this.

Motivation

See discussion at the end of #1798

Proposed solution

Revert the deprecation.

Copy link
Member

@ricardograca ricardograca left a comment

Choose a reason for hiding this comment

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

You should remove the import of deprecate since it's not used anymore, and add back the test for this feature that was also removed.

@@ -633,6 +633,9 @@ ModelBase.prototype.getTimestampKeys = function() {
* @param {string} [options.method]
* Either `'insert'` or `'update'` to specify what kind of save the attribute
* update is for.
* @param {string} [options.date]
* Either a Date object or ms since the epoch. Specify what date is used for
* the timestamps updated.
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this a little bit to make it clearer what the purpose of date is? Something like:

Specify what date is used as the value of now when creating or updating timestamp attributes. This overrides the current date with the specified value.

@ricardograca ricardograca added this to To Do in Version 0.14.0 via automation Apr 24, 2018
@ricardograca ricardograca moved this from To Do to In progress in Version 0.14.0 Apr 24, 2018
@mborst
Copy link
Contributor Author

mborst commented Apr 24, 2018

Updated.

@ricardograca ricardograca merged commit 304caf1 into bookshelf:master Apr 24, 2018
Version 0.14.0 automation moved this from In progress to Done Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants