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

Granular template checks for marketplace theme validation #518

Merged
merged 7 commits into from
Jun 2, 2021

Conversation

brandenrodgers
Copy link
Contributor

@brandenrodgers brandenrodgers commented May 28, 2021

Description and Context

Using this PR as the base just to split up the work into separate PR's to make it more reviewable.

This adds more accurate template checking to the theme validation command. Now we're correctly checking for label and screenshot annotations depending on the template type.

Screenshots

image

TODO

  • Add unit tests for the new template checks
  • This still needs more testing. I ran it against a marketplace theme and it seems to work, but there are a lot of edge cases that I could be missing.

Who to Notify

cc/ @KatzMitch

@brandenrodgers brandenrodgers changed the title Granular template checks for marketplace theme validation [WIP] Granular template checks for marketplace theme validation May 28, 2021

const TEMPLATE_LIMIT = 50;
const TEMPLATE_COUNT_IGNORE_LIST = ['global_partial', 'none'];
const VALIDATIONS_BY_TYPE = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Very cool

Base automatically changed from marketplace-theme-validators-2 to master June 2, 2021 16:03
@brandenrodgers brandenrodgers changed the title [WIP] Granular template checks for marketplace theme validation Granular template checks for marketplace theme validation Jun 2, 2021
@brandenrodgers brandenrodgers merged commit 6ca492b into master Jun 2, 2021
@brandenrodgers brandenrodgers deleted the marketplace-theme-validators-3 branch June 2, 2021 19:35
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.

5 participants