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

Use specific version of easol-canvas gem #3

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

sgerrand
Copy link
Contributor

@sgerrand sgerrand commented Sep 8, 2022

💁 Blindly installing easol-canvas can have unintended consequences, e.g. v3.0.0 now requires Dart Sass. This version should probably be configurable via the action itself.

Blindly installing `easol-canvas` can have unintended consequences, e.g. v3.0.0 now requires Dart Sass.
@sgerrand sgerrand added the bug Something isn't working label Sep 8, 2022
@sgerrand sgerrand merged commit f5f1127 into main Sep 8, 2022
@sgerrand sgerrand deleted the easol-canvas-lock-gem-version branch September 8, 2022 12:15
ianmooney added a commit that referenced this pull request Nov 25, 2022
The latest version of canvas is v3.1.0, which allows the
currency_switcher tag when validating Liquid.

Since #3, we are
now specifying the exact version of the `easol-canvas` gem.
This means there will be some extra steps when there is a an update
the the canvas gem. Each time there is an update, we'll need to do
the following:

1. Bump the `easol-canvas` gem version.
2. Update `easolhq/canvas-linter-action` to use the new gem version.
3. Update the theme's `.github/workflows/lint.yml` to use the new
   version of canvas-linter-action.

This might be cumbersome and also means that existing themes that don't
update their canvas-linter-action version won't benefit from the latest
validation checks.

This commit proposes we make the gem version more generic and only
require this linter action to to be updated when there major version
changes on the gem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

2 participants