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

May I request permission to merge updates of swc_core and publish new versions? #37

Open
kdy1 opened this issue Oct 13, 2023 · 15 comments

Comments

@kdy1
Copy link
Contributor

kdy1 commented Oct 13, 2023

For the context, I need to update swc_core quite frequently but updating swc_core of mdxjs-rs takes lots of time.

swc-project/swc#8116

@ChristianMurphy
Copy link
Contributor

Hey @kdy1 👋
@wooorm is traveling right now and may be slow to respond for a bit.

I need to update swc_core quite frequently but updating swc_core of mdxjs-rs takes lots of time.

I'm not quite sure I follow how this connects to the request for merge and release access.
If it takes time to update swc_core inside mdxjs-rs now, at a code level, it still will no matter who the merger or releaser is.

Taking a step back to the RFC, it sounds like the root issue is upstream

I need to update swc_core of next-swc periodically, but if there's a breaking change of swc_core, I have to update mdxjs-rs and swc-project/plugins.

SWC is making breaking changes regularly in minor and patch releases.


To summarize, @wooorm is the owner/releaser and would make the final decision.
I'd all for adding another merger/releaser, but the reasoning feels a bit like pushing frustration you are experiencing with SWC on the MDX project.

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 13, 2023

Typically swc_core update is simply modifying swc_core = "" in Cargo.toml.

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 13, 2023

It takes long time because I have to wait for a review

@jridgewell
Copy link
Collaborator

Hi @ChristianMurphy,

For reference, @kdy1 works for Vercel, and we frequently update this project in unison with updates in Next.js/Turbopack. Both myself and @kwonoj (also Vercel employees) have been added as contributors and publishers for this project to help make this maintenance a bit easier.

@ChristianMurphy
Copy link
Contributor

It takes long time because I have to wait for a review

We may have different definitions of "long time" looking through PR history it appear to usually be less than a day.
I'd consider that quite acceptable, even fast, given the team is scattered across different time zones.

For reference, @kdy1 works for Vercel, and we frequently update this project in unison with updates in Next.js/Turbopack. Both myself and @kwonoj (also Vercel employees) have been added as contributors and publishers for this project to help make this maintenance a bit easier.

Thanks for the context @jridgewell! More support is of course welcome.
Reading between the lines of your comment a bit, I am hearing you (@jridgewell) and @kwonoj are no longer have time or availability to supporting reviewing and cutting releases?

@jridgewell
Copy link
Collaborator

We're in US timezone, so usually asleep when @kdy1 opens the PRs. I'm happy to do it, but he still has to wait for me to start my next working day before I can.

@wooorm
Copy link
Owner

wooorm commented Oct 16, 2023

I’m fine adding access but I think it’s useful to get a review in from someone?

I (Europe/Amsterdam) typically release in what I think is a rather quick time?
And as two Vercel members in America/? already have release access, and could have released but didn’t yet, it doesn’t seem too long to me?

If swc is released, and this project was immediately released too, doesn’t it still take some time to review next/turbopack/etc? What sort of time is acceptable for that?

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 16, 2023

The problem is that, I can't even make PRs ready for review because it depends on two versions of swc_core.
So I have to pause the work, and resume tomorrow

@ChristianMurphy
Copy link
Contributor

I'd like to take a step back and express my hesitation, hoping that it might stem from a misunderstanding.

If mdxjs_rs is indeed a priority for @kdy1 and the Vercel team, it could potentially impede your releases. So, the question that arises is, why isn't mdxjs_rs included in the integration suite for SWC to prevent any breaking changes from occurring in the first place? You can find more information here: https://twitter.com/swc_rs/status/1716113290361991334 and https://github.com/swc-project/swc-ecosystem-ci/tree/758d592af487cc1b191fce9041944b32eff2c98a/tests.

The apparent gap, along with the requests and feedback from the Vercel team, could be interpreted by a community member as follows: "The Vercel team intends to break your project without prior notice. The Vercel team will only address the issue if it's convenient for them, without considering the downstream non-Vercel adopters who might be affected by breaking changes in mdxjs_rs, released without any review. Furthermore, timing the release to ensure other maintainers are unavailable to offer support if anything goes wrong."

This approach is certainly one way to handle open-source projects, but it doesn't seem very open or community-driven.

I genuinely hope that this is a misunderstanding, and there might be some missing context from the ongoing conversation. @kdy1 and the Vercel team generally appear to be well-meaning and community-driven collaborators. Can someone provide some clarity on this matter?

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 24, 2023

Becasue it's not depending on @swc/core npm package

There's no way to avoid breaking changes of Rust crates without restricting API severely.

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 24, 2023

SWC exposes extreme amount of APIs to Rust users to allow all ways of using SWC

@ChristianMurphy
Copy link
Contributor

ChristianMurphy commented Oct 24, 2023

So, it is not a misunderstanding, and all of that interpretation in #37 (comment) is correct?
That is not particularly reassuring @kdy1 😅

The concern is not the API surface in Rust, but how SWC interacts with other projects.
From a technical perspective, canary testing can also be done in Rust, the same as it can in JS.
Canary testing would likely ease a lot of pain, not just for mdxjs_rs, but other rust adopters as well.

From a people perspective, having reviews and some coordination around the review process between the maintainers and collaborators feels like the go-to open source approach. The push back I'm hearing feels rather surprising.

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 25, 2023

I don't know how on earth my comments translate like that. I just explained why I can't configure something like swc-ecosystem-ci for Rust-side users.

@kdy1
Copy link
Contributor Author

kdy1 commented Oct 25, 2023

From a technical perspective, canary testing can also be done in Rust, the same as it can in JS.

No. This is not possible, at least under the current API policy. We allow customizing literally everything, except some orderings between compatibility transforms. There's no way to allow such customizing while doing canary testing.

@ChristianMurphy
Copy link
Contributor

No. This is not possible, at least under the current API policy. We allow customizing literally everything, except some orderings between compatibility transforms. There's no way to allow such customizing while doing canary testing.

It is completely possible to canary test, there is no technical limitation that prevent it.
Furthermore, the lack of a cohesive API policy make it all the more important to do canary test to avoid breaking projects which are relying on a SemVer stable release.

There's no way to avoid breaking changes of Rust crates without restricting API severely.

There is a difference between public and private, stable and unstable.
I'm aware, and appreciate that you make APIs publicly accessible.
That does not mean everything has to be marked unstable.

I don't know how on earth my comments translate like that. I just explained why I can't configure something like swc-ecosystem-ci for Rust-side users.

My comment in #37 (comment) covered multiple testing, community, and release concerns.
Only canary testing has been even partially addressed, and I respectfully disagree with how it has been.
The rest remain concerns, your willful avoidance of acknowledging and addressing them comes across as tacit agreement they are real problems.

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

No branches or pull requests

4 participants