Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Update the API version to 2022-01 #1059

Merged
merged 3 commits into from
Feb 4, 2022
Merged

Conversation

Kyon147
Copy link
Collaborator

@Kyon147 Kyon147 commented Jan 17, 2022

This updates the default API version to be 2022-01 because 2021-01 is no longer supported.

It will flag up deprecation an error for apps like so:

image

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Jan 17, 2022

Hey @osiset

Would you know why composer normalize is failing on PHP8?

I accidentally pushed this to master (my total bad) and have reverted it on the master branch but the revert and this new PR are failing at this point.

Also, to stop my mistake from happening - is it worth making PRs and branches head into a different branch that is no master like a release candidate in future? Stops the stupid human error I just made...

@gnikyt
Copy link
Owner

gnikyt commented Jan 17, 2022 via email

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Jan 17, 2022

Yeah develop makes sense, as an RC can be made off that when there's enough reason to bump up to a version or patch etc.

Let me know if you have any insight on the composer normalize failing - not seen this error before...

@gnikyt
Copy link
Owner

gnikyt commented Jan 19, 2022 via email

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Jan 19, 2022

The only error I see is Error: Process completed with exit code 1. but on my local I am using php8.0 and composer normalize --dry-run run perfectly fine.

I can do a little more digging and see if the is an error being silenced further up the stack.

@gnikyt
Copy link
Owner

gnikyt commented Jan 19, 2022 via email

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Jan 19, 2022

Yeah, I think that makes sense - if it is a bug in the package maybe and for a check it is only "prettying" something rather than doing anything to stop a bug from getting introduced so seems safe to turn it off for now and keep tabs on if the package is updated.

What do you think?

As there is a potential bug for this test on PHP8.0 it will only run for older versions.
@Kyon147
Copy link
Collaborator Author

Kyon147 commented Jan 20, 2022

Hey @osiset

Removed the test step for normalizing on php8.0 only and the tests are running successfully now.

This PR has both the workflow update and the Shopify API minimum update together which should bring the package back into "pass" when deployed.

I'll leave it up to you to merge to master when you are happy 👍

@Kyon147
Copy link
Collaborator Author

Kyon147 commented Feb 1, 2022

Hey @osiset

Happy for me to merge this if you don't have the time?

@Kyon147 Kyon147 merged commit baa6248 into master Feb 4, 2022
@Kyon147
Copy link
Collaborator Author

Kyon147 commented Feb 4, 2022

@osiset - hope you don't mind I merged this in to fix the API default and the annoying test failure.

@gnikyt
Copy link
Owner

gnikyt commented Feb 4, 2022

@Kyon147 all good!

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

Successfully merging this pull request may close these issues.

None yet

2 participants