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.

Sign up for GitHub

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

Jump to bottom

[LiquidDoc][Theme Check] Add alias support for MissingRenderSnippetParams #830

Merged
merged 2 commits into from
Feb 28, 2025
Merged

[LiquidDoc][Theme Check] Add alias support for MissingRenderSnippetParams #830

merged 2 commits into from
Feb 28, 2025

Conversation

jamesmengo
Copy link
Contributor

@jamesmengo jamesmengo commented Feb 27, 2025 •
edited
Loading

What are you adding in this PR?

Closes https://github.com/Shopify/developer-tools-team/issues/604

What's next? Any followup issues?

  • Fix this for UnrecognizedParams (a little more involved for the suggested fix there)

Before you deploy

  • I included a minor bump changeset
  • My feature is backward compatible

@jamesmengo jamesmengo mentioned this pull request Feb 27, 2025
Merged
6 tasks
@jamesmengo Graphite App
Copy link
Contributor Author

jamesmengo commented Feb 27, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jamesmengo jamesmengo mentioned this pull request Feb 27, 2025
Merged
2 tasks
@jamesmengo jamesmengo changed the base branch from jm/split_param_checks to graphite-base/830 February 27, 2025 20:39
@jamesmengo jamesmengo changed the base branch from graphite-base/830 to jm/split_param_checks_split February 27, 2025 20:39
@jamesmengo jamesmengo mentioned this pull request Feb 27, 2025
Merged
@jamesmengo jamesmengo changed the title Add alias syntax support for MissingRenderSnippetParamsCheck - Update check to report + consider missing parameters when using 'as' alias - Adjust fixer to insert missing parameters at correct position [LiquidDoc] Add alias support for MissingRenderSnippetParams Feb 27, 2025
@jamesmengo jamesmengo added the #gsd:44310 LiquidDoc label Feb 27, 2025 — with Graphite App
@jamesmengo jamesmengo marked this pull request as ready for review February 27, 2025 20:58
@jamesmengo jamesmengo requested a review from a team as a code owner February 27, 2025 20:58
@jamesmengo jamesmengo requested review from aswamy and removed request for a team February 27, 2025 20:58
@jamesmengo jamesmengo changed the title [LiquidDoc] Add alias support for MissingRenderSnippetParams [LiquidDoc][Theme Check] Add alias support for MissingRenderSnippetParams Feb 27, 2025
@jamesmengo jamesmengo changed the base branch from jm/split_param_checks_split to graphite-base/830 February 27, 2025 22:11
@jamesmengo jamesmengo force-pushed the jm/aliases_missing_render_snippets branch from 2a2885f to 79481f6 Compare February 27, 2025 22:13
@jamesmengo jamesmengo changed the base branch from graphite-base/830 to main February 27, 2025 22:14
@jamesmengo jamesmengo force-pushed the jm/aliases_missing_render_snippets branch from 79481f6 to 6ce5eff Compare February 27, 2025 22:14
aswamy
aswamy approved these changes Feb 27, 2025
View reviewed changes
Copy link
Contributor

@aswamy aswamy left a comment

Choose a reason for hiding this comment

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

Functionally we should be good. I tried every iteration of with + as, with + for, and with standalone.

image

I did notice some funky warning with ValidRenderSnippetParamTypes for some reason, but don't think it's because of this ticket.

@jamesmengo
Add alias syntax support for MissingRenderSnippetParamsCheck
0d96194 
- Update check to report + consider missing parameters when using 'as' alias
- Adjust fixer to insert missing parameters at correct position
@jamesmengo
Update typo in test
013ca06
@jamesmengo jamesmengo force-pushed the jm/aliases_missing_render_snippets branch from 6ce5eff to 013ca06 Compare February 27, 2025 22:59
@jamesmengo jamesmengo mentioned this pull request Feb 28, 2025
Merged
2 tasks
@jamesmengo Graphite App
Copy link
Contributor Author

jamesmengo commented Feb 28, 2025 •
edited
Loading

Merge activity

  • Feb 28, 3:06 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Feb 28, 3:06 PM EST: A user merged this pull request with Graphite.

@jamesmengo jamesmengo merged commit 0bfe28f into main Feb 28, 2025
7 checks passed
@jamesmengo jamesmengo deleted the jm/aliases_missing_render_snippets branch February 28, 2025 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aswamy aswamy

Assignees
No one assigned
Labels
#gsd:44310 LiquidDoc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jamesmengo @aswamy