Skip to content

Conversation

@dmsnell
Copy link
Member

@dmsnell dmsnell commented Apr 21, 2023

Trac ticket #58170-trac

During the attempted merge of WordPress/gutenberg#49966 the PHP linting rule rejected the PR, even though it's a direct copy from Core.

In this PR we're adding spacing so that we can merge the "blessed" PR without creating conflicts between the two repositories.

@dmsnell dmsnell changed the title HTML API: Changes alignment to see if this passes rules; rejected currently rejected in GB HTML API: Update code style so it passes when backported into Gutenberg Apr 21, 2023
@dmsnell dmsnell marked this pull request as ready for review April 21, 2023 11:15
Copy link

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! ✅

@dmsnell dmsnell closed this Apr 21, 2023
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Apr 21, 2023
 - Support a few extra invalid comment syntaxes. WordPress/wordpress-develop#4256
 - Invalidate bookmarks which have been eliminated by enqueued changes. WordPress/wordpress-develop#4116
 - Expose whether the currently-matched tag has the self-closing flag. WordPress/wordpress-develop#4266
 - Avoid double-writing an attribute value if given case-variations of the name. WordPress/wordpress-develop#4337
 - Linting updates from WordPress/wordpress-develop#4360.
@dmsnell
Copy link
Member Author

dmsnell commented Apr 21, 2023

@hellofromtonya this apparently was fine in Core but in Gutenberg it was rejected. I'm not sure why or what happened, as the line in question was called out in review as a surprise that Core's linting didn't catch it. To the best of my recollection I made this change and submitted it only to find the Core linting rejected it, but maybe I messed up when I did that check, because it passes now.

Somewhere in here our linters don't agree, and I figured you might be the person to notify about it.

@dmsnell dmsnell deleted the html-api/lint-convergence-with-gutenberg branch April 21, 2023 13:03
@peterwilsoncc
Copy link
Contributor

Committed in r55674 / fea66be

dmsnell added a commit to WordPress/gutenberg that referenced this pull request Apr 24, 2023
 - Support a few extra invalid comment syntaxes. WordPress/wordpress-develop#4256
 - Invalidate bookmarks which have been eliminated by enqueued changes. WordPress/wordpress-develop#4116
 - Expose whether the currently-matched tag has the self-closing flag. WordPress/wordpress-develop#4266
 - Avoid double-writing an attribute value if given case-variations of the name. WordPress/wordpress-develop#4337
 - Linting updates from WordPress/wordpress-develop#4360.
 - Avoid losing previous updates in certain cases when seeking earlier in a document. WordPress/wordpress-develop#4345
@anton-vlasenko
Copy link

I wonder how the previous version of that file (before your patch) could possibly pass the Core CI checks? When I test this in my local environment, I get linter errors.

@dmsnell
Copy link
Member Author

dmsnell commented Apr 25, 2023

sounds like a good question @anton-vlasenko 🤷‍♂️

@peterwilsoncc
Copy link
Contributor

peterwilsoncc commented Apr 26, 2023

@anton-vlasenko @dmsnell The linter throws a warning rather than an error.

In WP-Dev, I believe the tests directory action fails on both warnings and errors, but the src directory only fails on errors. I guess the GB repo fails for both warnings and errors too.

Edit: Slowly src is getting closer to following the WP CS, I imagine failures on warnings will be enabled once the project is complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants