-
Notifications
You must be signed in to change notification settings - Fork 14
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
Added a DeprecatedTags
check
#50
Conversation
a78d864
to
459907c
Compare
DeprecatedTags
check
7143cb3
to
9ab0e9b
Compare
'@shopify/theme-check-docs-updater': minor | ||
--- | ||
|
||
Add `DeprecatedTags` check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad to see this working for ya now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you add the doc url here, we can get this merged. All other code changes LGTM.
Then you'd just need to do the fast follow task. Lemme know if you need help setting that up.
meta: { | ||
code: 'DeprecatedTags', | ||
name: 'Deprecated Tags', | ||
docs: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the doc url
, may we add: https://shopify.dev/docs/themes/tools/theme-check/checks/deprecated-tags
here?
As a fast followup, you will also need to do some light housekeeping on the dev docs.
Basically what we need is:
- convert-include-to-render docs copied for a new link address
https://shopify.dev/docs/themes/tools/theme-check/checks/deprecated-tags
with light modifications. - The old convert-include-to-render docs should get a new banner saying that the check has been migrated to typescript as deprecated-tags (with a link to the new documentation)
Spoke with Harib and he would like to wait on some followup input from CP regarding some bonus objectives for this change.
Spoke to CP regarding the bonus requirements for this check. He mentioned it being overly complicated at the moment, and that we would get back to it when time permits. |
bcc3940
to
12c794a
Compare
Resolves https://github.com/Shopify/theme-check-js/issues/47
What are you adding in this PR?
This PR is adding a
DeprecatedTags
check (for more context on why I took this approach, please read this comment). Under the deprecated tags check, we can add specific cases to deprecated tags that we would like to check for. This PR adds theConvertIncludeToRender
check which essentially converts a liquid tag that usesinclude
to userender
asinclude
is deprecated.What's next? Any followup issues?
If we need to check for any other deprecated tags, it should be written under this check as a specific case.
TopHat Screenshots
Before you deploy
changeset
allChecks
array insrc/checks/index.ts
yarn update-configs
and committed the updated configuration files