search

WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
207 stars 38 forks source link

#242 #260

Open WraithKenny opened 3 years ago

WraithKenny commented 3 years ago

Doing external file gets for a version number that hasn't changed in 4 years is more than a waste, it creates crashes, when all we need to do is update the static if it ever changes.

WraithKenny commented 3 years ago

Fixes #242

dingo-d commented 3 years ago

I personally don't have anything against it. @jrfnl what was the idea behind this sniff?

dingo-d commented 3 years ago

@WraithKenny can you just rebase this PR so that we see if the checks are passing on GH Actions? Thanks

WraithKenny commented 3 years ago

@dingo-d I merged lastest into patch-1

jrfnl commented 3 years ago

I personally don't have anything against it. @jrfnl what was the idea behind this sniff?

@dingo-d By the looks of it, the sniff itself hasn't changed, it's just the external call to the GH API to get the version number of the latest release of TGMPA which is being removed.

I'm fine with that (for now). At the time I added it, it was expected that there would be more regular TGMPA releases. The problem this code solved was that the sniff would have to be updated after every TGMPA release to make sure it would check for the latest release and then, of course, users would need to update their install of TRTCS regularly as well, as otherwise the sniff would still check against outdated version numbers. Removing this code, reintroduces that problem, but as stated above, there hasn't been a release of TGMPA for a number of years, so in practice, it's not a big thing at this moment.

If I ever find the time to do more regular maintenance of TGMPA again and we'd be releasing regularly again, we can always revisit and re-introduce this code.

dingo-d commented 3 years ago

@WraithKenny there is just one small error I'm seeing in the actions run

FILE: WPThemeReview/Sniffs/Plugins/CorrectTGMPAVersionSniff.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 203 | ERROR | Empty line not required before block comment
     |       | (Squiz.Commenting.BlockComment.HasEmptyLineBefore)
----------------------------------------------------------------------

Can you fix this and then I can merge this PR 👍🏼 .