New DateTime.CurrentTimeTimestamp sniff #1808
Conversation
|
@Rarst Looking at your blogpost at Make, should this sniff also check for |
Uh... Maybe later. We hadn't scraped those out of core yet. Want to just let release go out and dust settle for a bit right now. :) |
That shouldn't really be a consideration. As long as we don't make the error/warning auto-fixable, the Core builds won't fail on it (and it will help detect the issue in Core). |
Well, it is for me. Getting these things out of core is a necessary legwork to figure out what depends on it and what needs to be done to dump it in all (or at least a lot of) cases. My opinion is that we hadn't done equivalent amount of work to get rid of that one and are not ready to start sniffing it out. Soft recommendation to start using replacements takes a lower level of confidence than a hard sniff for me. |
Ok, fair enough. We can always add it at a later point in time. |
| * | ||
| * Disallow using the current_time() function to get "timestamps" as it | ||
| * doesn't produce a *real* timestamp, but a "WordPress timestamp", i.e. | ||
| * a Unix timestamp with current timezone offset, not a Unix timestamp ansich. |
There was a problem hiding this comment.
This new sniff adds a check for use of current_time() to retrieve a timestamp. A (fixable) `error` will be thrown when the `$gmt` parameter is set to `true` or `1`, a `warning` when it is not. Includes unit tests. Includes fixer. Includes documentation. This new sniff has been added to the `Core` ruleset. Fixes 1791
jrfnl
force-pushed
the
feature/new-currenttime-timestamp-sniff
branch
from
1dcfae7
to
e4f838c
Compare
|
Rebased for merge conflict. |
|
Anything I can do to move this PR forward ? |
This new sniff adds a check for use of current_time() to retrieve a timestamp.
A (fixable)
errorwill be thrown when the$gmtparameter is set totrueor1, awarningwhen it is not.Includes unit tests.
Includes fixer.
Includes documentation.
This new sniff has been added to the
Coreruleset.Fixes #1791
/cc @Rarst