Skip to content

Conversation

@scruffian
Copy link
Contributor

Backports WordPress/gutenberg#58975 to core.

I haven't yet been able to run the tests to check it works.

Trac ticket:


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions
Copy link

Trac Ticket Missing

This pull request is missing a link to a Trac ticket. For a contribution to be considered, there must be a corresponding ticket in Trac.

To attach a pull request to a Trac ticket, please include the ticket's full URL in your pull request description. More information about contributing to WordPress on GitHub can be found in the Core Handbook.

@github-actions
Copy link

github-actions bot commented May 22, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props scruffian, isabel_brison, ellatrix.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ellatrix ellatrix requested a review from tellthemachines May 29, 2024 08:20
Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

I'm not sure there's much benefit to syncing this to core, because these tests really need a wider refactor. I proposed that in WordPress/gutenberg#60842 (feedback welcome!) and intend to work on it in the next weeks.

The fact that this PR already has conflicts after a few days of being open illustrates the problem - every time we make updates to theme.json styles, these test strings also need to be updated, and the updates inevitably conflict 😬

That said, I don't have anything against this going into core, and am happy to help out with reviewing/committing once conflicts are solved!

Edit: if we go forward with this it also needs a trac ticket.

@ellatrix
Copy link
Member

ellatrix commented Jun 3, 2024

Let's keep this for the end, when all the other backports touching these tests are in.

@tellthemachines
Copy link
Contributor

Given #6734 removes base styles from all tests where they aren't relevant, is this change still needed?

@scruffian
Copy link
Contributor Author

Yeah this isn't needed anymore

@scruffian scruffian closed this Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants