Skip to content

Bump the WPCS version#239

Merged
jrfnl merged 1 commit intodevelopfrom
feature/update-package-versions
Nov 17, 2019
Merged

Bump the WPCS version#239
jrfnl merged 1 commit intodevelopfrom
feature/update-package-versions

Conversation

@dingo-d
Copy link
Copy Markdown
Member

@dingo-d dingo-d commented Nov 16, 2019

No description provided.

@dingo-d dingo-d added the meta Everything to do with the repo structure and organisation. label Nov 16, 2019
@dingo-d dingo-d added this to the 0.2.1 milestone Nov 16, 2019
@dingo-d dingo-d requested a review from jrfnl November 16, 2019 19:36
@dingo-d dingo-d self-assigned this Nov 16, 2019
@dingo-d dingo-d mentioned this pull request Nov 16, 2019
9 tasks
Copy link
Copy Markdown
Contributor

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

The ruleset should be updated for the deprecation of the WordPress.WP.TimezoneChange sniff.

For reference:

N.B.: nearly all of the new sniffs should probably be added to the theme review ruleset (like EscapedNotTranslated, ValidPostTypeSlug and the DateTime sniffs). This should be done in separate PRs, but those can be pulled as soon as this PR has been merged.

@dingo-d
Copy link
Copy Markdown
Member Author

dingo-d commented Nov 17, 2019

I like the idea, let me open up a new PR for this 👍

@jrfnl
Copy link
Copy Markdown
Contributor

jrfnl commented Nov 17, 2019

I like the idea, let me open up a new PR for this +1

👍 I suggest for the update related to the deprecated sniff to go in this PR though as the old sniff will start throwing deprecation notices as of WPCS 2.2.0.

@dingo-d
Copy link
Copy Markdown
Member Author

dingo-d commented Nov 17, 2019

I suggest for the update related to the deprecated sniff to go in this PR though

You mean the WordPress.WP.TimezoneChange sniff? I should replace it with WordPress.DateTime.RestrictedFunctions in the ruleset (from what I've read in the changelog).

@jrfnl
Copy link
Copy Markdown
Contributor

jrfnl commented Nov 17, 2019

I suggest for the update related to the deprecated sniff to go in this PR though

You mean the WordPress.WP.TimezoneChange sniff? I should replace it with WordPress.DateTime.RestrictedFunctions in the ruleset (from what I've read in the changelog).

It should (for now) be replaced by WordPress.DateTime.RestrictedFunctions.timezone_change_date_default_timezone_set.

The WordPress.DateTime.RestrictedFunctions sniff contains a second function group, so we need to make sure that one is not (yet) included.

Replacing the old sniff with the new code is one thing.
Adding additional checks is another.

@dingo-d dingo-d force-pushed the feature/update-package-versions branch from 17a2b4a to baae535 Compare November 17, 2019 11:12
@jrfnl
Copy link
Copy Markdown
Contributor

jrfnl commented Nov 17, 2019

@dingo-d Is it me or did you add additional new sniffs to this PR now anyway ? Those are new decision points and would be a reason to reject this PR.

@dingo-d
Copy link
Copy Markdown
Member Author

dingo-d commented Nov 17, 2019

Oh, I thought that I'd put the new sniffs with the deprecation ones since they all come with the new version of the WPCS 🤷‍♂

I can remove them and open a new PR with those changes

Bumped the version of the WPCS in the composer.json. Replaced the deprecated rule WordPress.WP.TimezoneChange with WordPress.DateTime.RestrictedFunctions.timezone_change_date_default_timezone_set rule.
@dingo-d dingo-d force-pushed the feature/update-package-versions branch from baae535 to 8aa0e31 Compare November 17, 2019 15:48
@jrfnl
Copy link
Copy Markdown
Contributor

jrfnl commented Nov 17, 2019

@dingo-d Thanks for removing those. Once the build passes, I'll merge this.

@jrfnl jrfnl merged commit f395699 into develop Nov 17, 2019
@jrfnl jrfnl deleted the feature/update-package-versions branch November 17, 2019 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Everything to do with the repo structure and organisation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants