-
Notifications
You must be signed in to change notification settings - Fork 29
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
Github Actions and PHPCS updates #108
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🌵
@@ -190,9 +199,10 @@ function test_wp_query_paged_and_posts_per_page() { | |||
} | |||
|
|||
/** | |||
* @ticket 18897 | |||
* @ticket https://core.trac.wordpress.org/ticket/18897 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ticket
is not actually a valid phpDoc.
* @ticket https://core.trac.wordpress.org/ticket/18897 | |
* @link https://core.trac.wordpress.org/ticket/18897 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ticket
is a special annotation that the core phpunit test suite adds. If I recall correctly (and I may not, it's been a long time!), based on the status of the ticket, this test would either be run or skipped automatically. Was that feature not working on GitHub Actions, or did the test start failing after the annotation was changed to the full url? Or does core's test suite work differently now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't working to skip the tests in Github Actions or locally. I can investigate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently that feature was removed back in 2021. Womp womp.
@ticket
is still a valid annotation, but since it no longer functions in a useful way for us, I agree that @link
is better. If we want, we can still conditionally skip the test based on the status of the ticket by calling:
$this->knownWPBug( 18897 );
That might be better than always skipping it? 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mboynes TIL. Thank you for the link. 〽️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
Co-authored-by: Renato Alves <[email protected]>
This reverts commit 3bb279e.
This reverts commit ca43537.
This reverts commit 4d3c5d8.
Resolves #98