-
Notifications
You must be signed in to change notification settings - Fork 19
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
Automatically resolve to_shopify
mapping chain from oldest to current version
#517
Merged
danielpgross
merged 1 commit into
main
from
12-20-wip_resolve_to_shopify_mappings_from_intermediate_versions_to_latest
Jan 8, 2025
Merged
Automatically resolve to_shopify
mapping chain from oldest to current version
#517
danielpgross
merged 1 commit into
main
from
12-20-wip_resolve_to_shopify_mappings_from_intermediate_versions_to_latest
Jan 8, 2025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
danielpgross
force-pushed
the
12-20-wip_resolve_to_shopify_mappings_from_intermediate_versions_to_latest
branch
6 times, most recently
from
January 2, 2025 21:06
2cd98b0
to
e2f3857
Compare
danielpgross
changed the title
WIP: Resolve to_shopify mappings from intermediate versions to latest
Resolve Jan 2, 2025
to_shopify
mappings from old versions to latest
danielpgross
changed the title
Resolve
Automatically resolve Jan 2, 2025
to_shopify
mappings from old versions to latestto_shopify
mapping chain from oldest to latest version
danielpgross
changed the title
Automatically resolve
Automatically resolve Jan 2, 2025
to_shopify
mapping chain from oldest to latest versionto_shopify
mapping chain from oldest to current version
danielpgross
force-pushed
the
12-20-wip_resolve_to_shopify_mappings_from_intermediate_versions_to_latest
branch
from
January 2, 2025 21:47
e2f3857
to
2add753
Compare
elsom25
approved these changes
Jan 6, 2025
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.
Such an important change — left just a few stylistic nits, nothing blocking
danielpgross
force-pushed
the
12-20-wip_resolve_to_shopify_mappings_from_intermediate_versions_to_latest
branch
from
January 8, 2025 21:39
2add753
to
479060a
Compare
danielpgross
deleted the
12-20-wip_resolve_to_shopify_mappings_from_intermediate_versions_to_latest
branch
January 8, 2025 21:44
This was referenced Jan 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
Changes how integration mappings are defined from older Shopify taxonomy versions.
Before:
data
, old Shopify integration versions were defined relative to the current/latest version (ex. 2022-01 is defined relative to 2025-01-unstable)to_shopify
mapping, we needed to also add it individually to each older version.dist
, old Shopify integration versions were generated relative to the current versionAfter:
data
, old Shopify integration versions are defined relative to the immediately next version (ex. 2022-01 is defined relative to 2024-07)dist
, old Shopify integration versions are generated relative to the current versionHow it works
Category
objects using the current taxonomy. Instead, the raw category ID is stored.Category
objects, taking into account any mappings that appear in newer versions.Breakdown of changes
The PR is big, but most of the changes are test fixtures. The actual implementation changes are mostly in:
IntegrationVersion#load_all_from_source
IntegrationVersion#resolve_to_shopify_mappings
The other notable change is extracting out one of the tests from
IntegrationVersionTest
into a dedicated integration test (GenerateMappingsDistTest
). Instead of heavily relying on mocking like the old test, the new dedicated test compares generated file contents against fixtures. This approach is more straightforward and easier to maintain.