Fixes/type errors #1245
Fixes/type errors #1245
Conversation
|
Hi there 👋 Sorry for the sluggish responses on this. Tldr; we do want to get this merged. And we do appreciate you taking the time to find all these type errors. We have a ton of upstream friction with the REST resources, and we would ideally make these changes upstream, like we outline in our contribution guidelines. Though we are planning on merging this when the January API release goes out, and apply these changes. |
|
@lizkenyon A response! I never thought I'd see the day. Sorry I'm so salty about this, it's just that it isn't the first time I've tried to be a helpful contributor only to never even get an acknowledgement. Anyway, sure, fix the upstream. That does sound like the ultimate solution. But right now things are broken. I literally can't use anything but my fork. So while it may feel dirty to allow multiple types, but now it's the only solution. I just want you to know about the current ongoing pain this is causing. Please let me know if you need anything else. |
|
Thank you for your patience! :) Sorry to clarify, ideally we would fix the upstream changes, but right now it looks like we will not. If we merge this in today, in 3 weeks with the release of the January API version the REST resources would be regenerated, and would overwrite these changes. We would like to merge this, but we were thinking we would merge this after January 1st, so we would not have to reapply these changes again. Does that make sense? What are your thoughts on this approach? |
|
Yes, I feel you. You're kind of caught there. I understand now that the API is auto-generated based on the REST resources. So yeah, merging Jan 1st makes the most sense. 👍 |
Feature: Tests for FulfillmentEvent that use non-nil `province` and `country` attributes Q: Does the API ever return Integer? Hard to prove a negative Fix: Variant class `inventory_quantity` attribute type which can be either integer, string or nil Chore: Add to changelog
hrdwdmrbl
force-pushed
the
fixes/type_errors
branch
from
d4de6f4 to
ae56e50
Compare
|
@lizkenyon I already did sign the CLA. Just rebased and made the same changes to 2024-01 |
|
Thanks for your contribution! |
Since you completely ignored my last PR which contained one of these fixes, I assume that you will also ignore this one. But alas I submit this PR anyway. That one even had full tests and everything! In this one I didn't bother updating the tests with the latest fixes because I don't trust you to even look at this so why bother? Why bother having code of conduct and PR templates if no one even reviews the PRs? 🙄
Anyway, your types are wrong, please update them. Your own backend does not always return the types that you use here. You can pretend like it does, but it does not.
Shopify: 🙈🙉🙊
Me: 🤯