Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make parser warnings more clear #395

Merged
merged 14 commits into from
Jan 29, 2024
Merged

Make parser warnings more clear #395

merged 14 commits into from
Jan 29, 2024

Conversation

ernilambar
Copy link
Member

Fixes #394

@ernilambar
Copy link
Member Author

https://meta.trac.wordpress.org/ticket/7412 This will add more parser warnings. It'd be better if we incorporate those changes here also.

@ernilambar ernilambar force-pushed the 394-parser-warning-message branch 2 times, most recently from 55274b5 to a58a056 Compare January 23, 2024 04:55
@ernilambar ernilambar marked this pull request as ready for review January 23, 2024 06:35
@ernilambar
Copy link
Member Author

PR is updated and new warnings are also incorporated.

cc: @swissspidy @felixarntz @mukeshpanchal27

@ernilambar ernilambar mentioned this pull request Jan 24, 2024
Merged
@ernilambar ernilambar requested a review from swissspidy January 25, 2024 04:31
mukeshpanchal27
mukeshpanchal27 requested changes Jan 25, 2024
View reviewed changes
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @ernilambar for the PR. Great start. Left some feedback for your consideration.

Additionally.

  • Add unit tests that demonstrate all warnings that you listed.
  • Add unit test that ignore all these warnings. I think we can archive it using add_filter( 'wp_plugin_check_ignored_readme_warnings', '__return_empty_array' );.

@ernilambar ernilambar force-pushed the 394-parser-warning-message branch 2 times, most recently from 5e7bca5 to 6ea8955 Compare January 25, 2024 11:52
@ernilambar
Copy link
Member Author

ernilambar commented Jan 25, 2024 •
edited
Loading

  • Add unit test that ignore all these warnings. I think we can archive it using add_filter( 'wp_plugin_check_ignored_readme_warnings', '__return_empty_array' );.

Its the opposite. Sending empty array in that filter will disable ignoring and all warnings are displayed.
We already have test_filter_wp_plugin_check_ignored_readme_warnings_will_return_no_error to test same functionality. Do we need another similar one?

@ernilambar ernilambar force-pushed the 394-parser-warning-message branch from 6ea8955 to 3f2a463 Compare January 26, 2024 05:31
@ernilambar ernilambar force-pushed the 394-parser-warning-message branch 2 times, most recently from cec8e01 to f2ff3a2 Compare January 29, 2024 11:10
@ernilambar ernilambar force-pushed the 394-parser-warning-message branch from f2ff3a2 to 595cd97 Compare January 29, 2024 11:16
@mukeshpanchal27 mukeshpanchal27 merged commit a50f3f1 into WordPress:trunk Jan 29, 2024
24 checks passed
@ernilambar ernilambar deleted the 394-parser-warning-message branch January 29, 2024 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swissspidy swissspidy swissspidy approved these changes

@mukeshpanchal27 mukeshpanchal27 mukeshpanchal27 approved these changes

@felixarntz felixarntz Awaiting requested review from felixarntz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make more clear about readme parser warnings
3 participants
@ernilambar @swissspidy @mukeshpanchal27