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

Set labels for ATE Gen3 #347

Merged
merged 1 commit into from
Mar 11, 2024
Merged

Set labels for ATE Gen3 #347

merged 1 commit into from
Mar 11, 2024

Conversation

dgwatkins
Copy link
Collaborator

@dgwatkins dgwatkins changed the title Set labels for core/image strings [Draft] Set labels for core/image strings Jan 4, 2024
@dgwatkins dgwatkins marked this pull request as draft January 4, 2024 14:45
@dgwatkins dgwatkins marked this pull request as ready for review March 11, 2024 13:05
@dgwatkins
Copy link
Collaborator Author

I believe we can merge this already because we are just updating the labels.

@OnTheGoSystems/compatibility

@dgwatkins dgwatkins changed the title [Draft] Set labels for core/image strings Set labels for ATE Gen3 Mar 11, 2024
@dgwatkins
Copy link
Collaborator Author

Note that we don't need specific labels for every element because we try to put together the label automatically. We only set a specific label when the automatic label doesn't match the requirements.

Copy link
Member

@strategio strategio left a comment

Choose a reason for hiding this comment

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

@dgwatkins, I quickly scanned the changes and it looks good. The only question is about the order of the items in the config: Our current config does not respect a clear order (alphabetical or other), and in your changes you also changed the order of some items, but still it's not alphabetical. Did you opted for another specific order?

@dgwatkins
Copy link
Collaborator Author

dgwatkins commented Mar 11, 2024

No @strategio

As far as I can see, the order of the XML elements has nothing to do with it.
ATE receives strings in the order they are registered in (string_location field).

The problem of the string_location is that we only use it in page-builders.

I did change the order of divi shortcode definitions to confirm this (et_pb_cta).

I left it changed because it makes more sense to have the titles at the top and the rest below.

Copy link
Member

@strategio strategio left a comment

Choose a reason for hiding this comment

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

@dgwatkins dgwatkins merged commit d36c947 into master Mar 11, 2024
1 check passed
@dgwatkins dgwatkins deleted the wpmlpb-419 branch March 11, 2024 18:20
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.

2 participants