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

GitHub actions theme check fix #3702

Closed
wants to merge 2 commits into from

Conversation

Jyrko
Copy link

@Jyrko Jyrko commented Jan 14, 2025

PR Summary:

This problem fixes the Github Actions CI/CD problem described here (#3701).

Fixed the problem for internal development of the theme.

Why are these changes introduced?

to fix Github Actions CI/CD problem in order to pass tests.

What approach did you take?

  1. Start with research (check the official theme-check repository )
  2. Create a hypothesis (it may be related with the theme-check version, as v1 is outdated)
  3. Execute the experiment (try to change theme-check version)
  4. If it fails then refine the hypothesis (in this case it is not)

Other considerations

As the theme-check version was updated, then I needed to fix an error in header.liquid file in order tests to pass because of LiquidHTMLSyntaxError.

Decision log

# Decision Alternatives Rationale Downsides
1

Visual impact on existing themes

It will not

Testing steps/scenarios

  • Step 1

Checklist

@Jyrko
Copy link
Author

Jyrko commented Jan 14, 2025

I have signed the CLA!

@graygilmore
Copy link
Contributor

Thanks Jyrko! I've got another PR with some additional changes #3704 that we'll be using instead. Thanks for your contribution!

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