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

build: update shopify SDK #274

Merged
merged 3 commits into from
Oct 24, 2023
Merged

build: update shopify SDK #274

merged 3 commits into from
Oct 24, 2023

Conversation

ankush
Copy link
Member

@ankush ankush commented Oct 24, 2023

  • build: update shopify sdk
  • fix: product sync
  • fix: Correctly wrap function

Without this function signature is not passed along and causes frappe's
introspection magic to fail
@ankush ankush enabled auto-merge October 24, 2023 06:57
@ankush ankush merged commit ace843f into develop Oct 24, 2023
@ankush ankush deleted the update_sdk branch October 24, 2023 07:06

const { message: jobs } = await frappe.db.get_list("RQ Job", {filters: {"status": ("in", ("queued", "started"))}});

const jobs = await frappe.db.get_list("RQ Job", {filters: {"status": ("in", ("queued", "started"))}});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way we can run version-specific code from js? Because, this doesn't work in version-13 as it doesn't have RQ Jobs.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can check version from frappe.boot.versions (I don't remember actual key, but it should be there)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Got it.

parseInt(frappe.boot.versions.frappe); // returns 13, 14, or 15

Which is better? Calling separate functions based on the frappe version or having separate branches of this repo like version-13 and version-15?

Or is version-13 outdated enough to warrant an upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

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

V13 will be EOL in 2 months. Maintaining multiple branches is kind of a hassle.

Copy link
Member Author

Choose a reason for hiding this comment

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

Diff code based on condition is fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. But again, we only have one customer on v13 using this. I'm not sure if there are any others still on v13 with this app.

I could just fork this and keep the old code on it instead of changing the upstream for one single customer.

It will also prevent any future updates messing with v13.

@github-actions
Copy link

🎉 This PR is included in version 1.19.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants