search

MetaMask / action-npm-publish

GitHub Action to publish to NPM
MIT License
8 stars 6 forks source link

handle publish for yarn v1 #17

Closed legobeat closed 1 year ago

legobeat commented 1 year ago

This adds preliminary publishing for repos with yarn v1.

rickycodes commented 1 year ago

My thinking here was if we need to support the old way people can just consume those old tags (like they are doing in places today)

but we should probably document that usage if that's the case.

I'm also fine with supporting backwards compat if this get us there. Curious to hear what other team members think

mcmire commented 1 year ago

@legobeat Can you provide more context for this change? Originally this action did support Yarn v1, and then we made v3 a requirement in #10. I am trying to find where the shared libraries team talked about this change — that may happened on a call — but I agree with Ricky, and on top of that, I believe our thought was that we want to move everything to v3 anyway.

legobeat commented 1 year ago

The reasoning here is that this can be useful for repos where yarn3 is yet to be made/prioritized, and legacy packages where it may not be done - seeing as it's a trivial change. Note that the README is left with:

This action assumes that Yarn is installed and that the package is using Yarn v3. It may fail for other Yarn versions or other package managers.

If this is still up for consideration: PR title changed to reflect non-support; Added a warning for when Yarn Classic is used, indicating users should upgrade. 0b996e70f14064cbc1f8ec053fa7ed6539be3741

mcmire commented 1 year ago

If this is still up for consideration: PR title changed to reflect non-support; Added a warning for when Yarn Classic is used, indicating users should upgrade

I don't know that a lot of people will notice this warning, to be honest, since it appears in the CI output.

You mentioned the note in the README, but what if we remove that bit?

mcmire commented 1 year ago

@legobeat Coming back to this PR... I'm on the fence about this. On one hand, your solution is very reasonable and harmless. On the other hand, we already have a solution right now, in that projects that still use Yarn v1 can use an older version of this action. How many projects do we have that still use Yarn v1? I think the inconvenience caused by only supporting Yarn v3 is a feature and not a bug in this case because it serves as a reminder for us to upgrade our libraries. (And among all of the types of upgrades this one is probably the least painful.) So unless there is a new update that we need Yarn v1 projects to have that they wouldn't be able to get by sticking with an older version, I am kind of leaning toward going with the latter solution and keeping things as they are.

legobeat commented 1 year ago

@mcmire got it - will be backporting changes to a fork of v1 as necessary instead then!

legobeat commented 5 months ago

Reviving this again as maintenance of Yarn Classic packages is still ongoing and hours are wasted on identifying compatible action versions.

legobeat commented 5 months ago

Reviving this again as maintenance of Yarn Classic packages is still ongoing and hours are wasted on identifying compatible action versions.

Huh, it seems the a rebase somehow closed this PR and prevents reopening it. Making a new one.