-
Notifications
You must be signed in to change notification settings - Fork 41
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
Include a Base layer for Settings and Commerce fieldsets #2922
base: master
Are you sure you want to change the base?
Conversation
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.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
phpcs
src/Tickets/Site_Health/Fieldset/Settings.php|41 col 22| WordPress.Arrays.MultipleStatementAlignment.DoubleArrowNotAligned
Array double arrow not aligned correctly; expected 4 space(s) between "'id'" and double arrow, but found 1.
src/Tickets/Site_Health/Fieldset/Settings.php|86 col 1| PSR2.Files.EndFileNewline.NoneFound
Expected 1 newline at end of file; 0 found
src/Tickets/Site_Health/Provider.php|46 col 8| Squiz.Commenting.FunctionComment.EmptyThrows
Comment missing for @throws tag in function comment
src/Tickets/Site_Health/Provider.php|48 col 8| Squiz.Commenting.FunctionComment.MissingParamComment
Missing parameter comment
🗒️ Description
Include Telemetry data about how we are using Event Tickets to improve deprecation of old code and guide proper feature development.
It for sure doesn't include all the data points we want but it creates a small setup start pushing that data.
This separates the value of each one of our data-points in a way that we can add tests, which I haven't gotten to I ran out of time this Friday, only had about 1h to actually do code today.
Once we include the change to the Sniffer we should avoid problems with
@inheritDoc
stellarwp/coding-standards#22
🎥 Artifacts
https://i.bordoni.me/tNrzrWXQ
✔️ Checklist