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.

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

Update theme.json version #36917

Merged
merged 1 commit into from
Nov 30, 2021
Merged

Update theme.json version #36917

merged 1 commit into from
Nov 30, 2021

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Nov 26, 2021

Description

With #36155 the schema version needs to be updated to 2.

It looks like the v2 properties were updated without updating the version, so this is the only change needed.

Version 1 is not allowed because the new properties are not available in version 1. This will help folks see which properties need updating when switching to version 2.

How has this been tested?

See that version 2 theme.json files work with the schema

Types of changes

Fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@ajlende ajlende added [Type] Bug An existing feature does not function as intended Developer Experience Ideas about improving block and theme developer experience labels Nov 26, 2021
@ajlende ajlende self-assigned this Nov 26, 2021
Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

LGTM!

@ajlende ajlende merged commit 570751a into trunk Nov 30, 2021
@ajlende ajlende deleted the fix/theme-json-version branch November 30, 2021 06:41
@github-actions github-actions bot added this to the Gutenberg 12.1 milestone Nov 30, 2021
@@ -1077,7 +1077,7 @@
"version": {
"description": "Version of theme.json to use.\nSince 5.8.",
Copy link
Member

Choose a reason for hiding this comment

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

Hi, was looking and this and wondered if there a way to say:

  • this property version is present since 5.8
  • its value was changed to 2 in 5.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think trying to add notes to that level of detail would be too difficult to keep updated. We could also just remove those version notes completely since we have tagged versioning now. However, it's still kind of nice when using trunk to see when things were added without having to open all the previous versions.

Copy link
Member

@colorful-tones colorful-tones Dec 3, 2021

Choose a reason for hiding this comment

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

I do think that we need to update the "Version of theme.json to use.\nSince 5.8.", to be "Version of theme.json to use.\nSince 5.9.", correct? Because v2 of theme.json is not available until WordPress 5.9 release.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I think I see my confusion. We're describing the property introductory version. Super confusing!

We could also just remove those version notes completely since we have tagged versioning now.

I would vote for this based on my example of confusion. 🤦

@Mamaduka
Copy link
Member

Cherry-picked for 5.9.1.

@Mamaduka Mamaduka removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 16, 2022
Mamaduka pushed a commit that referenced this pull request Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants