ds300 / patch-package

Fix broken node modules instantly 🏃🏽‍♀️💨
MIT License
10.42k stars 294 forks source link

This lib should submit PRs to source repos #427

Open ajp8164 opened 2 years ago

ajp8164 commented 2 years ago

I like the sentiment of patch-package. I wish it would automagically generate a PR for the source repo though (on its own in a scratch space or wherever). To make this work we'd need a very well defined process to handle interactions prior to merge. This package does great at getting me going right now without having to fork etc (which always seems like a huge hassle, including follow ups etc). Seems to me that patch-package produces brittle apps and falls short in promoting "bug free" ;-) open source packages.

orta commented 2 years ago

Considering the complexity of that feature, (and you're usually editing built assets) I'm not super into the idea that it should live in patch package.

That said, I think we could have a command which re-uses the github cli to do some of that work potentially?

milahu commented 2 years ago

a first step would be https://github.com/milahu/patch-package/issues/13

ajp8164 commented 1 year ago

Considering the complexity of that feature, (and you're usually editing built assets) I'm not super into the idea that it should live in patch package.

That said, I think we could have a command which re-uses the github cli to do some of that work potentially?

Idea - the devs patch process can force the dev to patch original source and then use an automated process to create the runtime patch. As @milahu suggests, the use of a repo (essentially a behind the scenes form) seems fairly straightforward.

I'm my experience forking and submitting is a painful process and could be nearly fully automated. The goal is to encourage devs with an easier process to contribute changes. Next step is to decentralize repo ownership (blockchain style) to keep repos updated. Maintainers leave a lot of good repos and so getting a single core of maintainers out of the process would be ideal. IMHO :-)

milahu commented 1 year ago

decentralize repo ownership

deno is one way to get rid of NPM (because we cant get rid of typescript ...)

forking and submitting is a painful process

writing such a "killer app" is painful too, mostly because nobody will use it (marketing no demand)

abiriadev commented 3 months ago

I like the sentiment of patch-package. I wish it would automagically generate a PR for the source repo though (on its own in a scratch space or wherever). To make this work we'd need a very well defined process to handle interactions prior to merge. This package does great at getting me going right now without having to fork etc (which always seems like a huge hassle, including follow ups etc). Seems to me that patch-package produces brittle apps and falls short in promoting "bug free" ;-) open source packages.

I think it doesn't make much sense for the following reasons:

  1. PR is a feature of GitHub, not NPM nor Git.
    • What if the source of the package is on GitLab, a self-hosted Git, or even an auto-generated tar?
    • What if the package is published by someone who forked the original repo? (This is surprisingly common case) Where should the PR be directed?
  2. We use patch-package mostly because maintainers have refused to accept our PRs or improvements.
    • There are many reasons: the maintainer may have stopped maintaining, or even archived it, or the suggestion conflicts with the package's intended use, so on. patch-package is the 'last resort', and when we have to use it, it is likely that the maintainer has already rejected our PR more than once.
  3. Making a direct PR is the right way. NPM only contains the code necessary to run, not the original source.
    • What if the original repository only contains TypeScript code? We can only see and modify the compiled .js and .d.ts files, not the typescript source. How can we 'merge' our suggestion into the original repository under these cases?

which always seems like a huge hassle, including follow ups etc

Of course, I totally agree with you. But I can't think this is 'more proper' way than an ordinary fork-than-PR based process.