-
-
Notifications
You must be signed in to change notification settings - Fork 376
Conversation
Hey @osiset Would you know why 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... |
Hey! Yeah we can do a develop branch maybe?
…On Mon., Jan. 17, 2022, 11:17 Luke Walsh, ***@***.***> wrote:
Hey @osiset <https://github.com/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...
—
Reply to this email directly, view it on GitHub
<#1059 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASO4OWZZIVVFSNRI3PF4KLUWQTWRANCNFSM5ME3X2DA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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 |
It doesn't say an error does it? It says something about plugins needing to
be added by June 2022. I tried to look back at an older action for what it
normally outputs with, but it says logs are not available.
Is there an actual error you see there?
…On Mon, Jan 17, 2022 at 11:22 AM Luke Walsh ***@***.***> wrote:
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...
—
Reply to this email directly, view it on GitHub
<#1059 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASO4OUGGVQFBLFL4DTVBX3UWQUKBANCNFSM5ME3X2DA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Thanks,
Tyler King
|
The only error I see is I can do a little more digging and see if the is an error being silenced further up the stack. |
Should we just forward the error off for now? Seems like it may be a bug
with GH Actions, I've seen similar reports of people running into exit code
when it works fine on local.
…On Wed, Jan 19, 2022 at 10:57 AM Luke Walsh ***@***.***> wrote:
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.
—
Reply to this email directly, view it on GitHub
<#1059 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AASO4OSF7WWR2HO55EGVUH3UW3C6FANCNFSM5ME3X2DA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Thanks,
Tyler King
|
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.
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 👍 |
Hey @osiset Happy for me to merge this if you don't have the time? |
@osiset - hope you don't mind I merged this in to fix the API default and the annoying test failure. |
@Kyon147 all good! |
This updates the default API version to be
2022-01
because2021-01
is no longer supported.It will flag up deprecation an error for apps like so: