5 ms·

The End of Monkeypatching

28 points by pwim 16y ago

perlgeek 16y ago
It misses the most important step: submit a pull request to the project maintainer, so that you don't end up maintaining every project yourself for which you ever fixed a bug.
- keyle 16y ago
  I see so many pull requests that get no feedback and being totally ignored, it gets me wondering about this whole forking fanzy... For major projects it may work, but sometimes you're updating an old library to work with a newer version of a platform and get dead silence. It doesn't do anyone any good.
  - swaits 16y ago
    It's just laziness. I recently sent the ditz maintainers a patchset for 1.9 compatibility, which they promptly ignored. They aren't on github/bitbucket, so I had to manually diff a copy of the original code against mine in order to create the patch. Then I needed to subscribe to the mailing list and send them the patch. On their end, they would need to run patch to apply my changes. None of this is hard, we've been doing it for a long time. But at least with github/bitbucket, most of this friction is reduced to a few simple clicks. It's certainly no worse.
    - Someone 16y ago
      It's just laziness. [...] Then I needed to subscribe to the mailing list and send them the patch. On their end, they would need to run patch to apply my changes. If only things were that simple. A proper maintainer would have to: a) decide whether what this patch claims to do with the code moves the code in a direction where the maintainers want it to move. b) review the code for quality, security concerns and outright backdoors. c) possibly: compare it to a zillion patches that (more or less) claim to do the same. d) assuming the patch is perfect: check with collisions on other pending patches, recently committed changes, etc. I do not know about this project, and have no real expertise as a maintainer, but I am fairly sure it is not "just laziness". If it were, all projects would have an email address that blindly applies each patch sent to it to the code base.
      - swaits 16y ago
        > I do not know about this project, and have no real expertise as a maintainer In my example case, it certainly is. There certainly aren't zillions of patches. The maintainers are just not "maintaining". Plain and simple.
  - zacharyz 16y ago
    The worst is when you find a project that isn't being maintained but still solves a problem. Then you have to trace through all the different pull requests to find what actually works and is relavent to your needs.
yxhuvud 16y ago
Yes, instead of getting support hell for monkeypatching, we get support hell for using forked versions of the libs. Consider me skeptical that there will be any real improvement from this. (I am not claiming forks are bad, only that it solves a different problem than support hell)
- michaelfeathers 16y ago
  Agreed. There should be a term for "takes problem from one place and puts it in another place yielding no net advantage."
  - JonnieCache 16y ago
    "Shuffling the deckchairs on the titanic" comes to mind, but it isn't really a term.
    - IsaacL 16y ago
      I've heard that phrase a few times before, but I don't think it fits the context here. The context is a "solution" that creates more problems than it solves. "Shuffling the deckchairs/polishing the silverware on the Titanic" refers to busying oneself with small problems when there's a honking great problem that's being ignored.
  - riffraff 16y ago
    parasolving? Though this is not really the case, I'd say it's more like "the problem is still there but we walked around it with a different route". Perisolving maybe? Especially if you consider that part of the problem is that something won't work but it may take hours to understand it's because the semantics of Enumerable#group_by in library X are not the same as the ones defined in library Y.
  - goodside 16y ago
    Circumsolution.
  - Confusion 16y ago
deleted 16y ago
[deleted]
grammaton 16y ago
Monkey patching is bad software engineering, but it happens because it also works in many cases, and because it's fast. The fork and fix approach is a pretty heavy weight solution to an often trivial problem. Just because git (and lets face it, ruby and git go hand in hand) lets you branch to your heart's content, doesn't mean you should.
mckoss 16y ago
The key benefit can be had with monkey patching as well: - Publishing a new solution and making it available. The packaging is not ideal, as setup and configuration of the lib+patch will not be as simple as pulling a modified lib. But think what you would have to do to incorporate 5 separate patches to a given library. It could very well be simpler to run 5 monkey patches than to fork a project and do 5 merges.
bgentry 16y ago
I wish there was more of an emphasis on this being a temporary solution. Usually when you need a patch, you need it now, so pulling the lib from your own git fork is the only immediate solution. But you should make every effort with your fork to adhere to the project's standards and get your fixes merged back in so the entire ecosystem can benefit. Get off the custom gem as soon as possible.
MichaelGG 16y ago
Non-Ruby user here. I thought monkeypatching was replacing members at runtime, not just adding them. Or adding them to an instance of the object instead of the class. Isn't just adding members like a "type extension" that statically typed languages can safely do? (Not flaming, just trying to understand the differences.)
- awj 16y ago
  That actually is (sort of) addressed in the article's initial example. Your class extension can turn someone else's class extension into a monkey patch. Worse, your extension may not match calling semantics of the other extension, so you can get some truly ugly behavior.
djacobs 16y ago
I really don't like this solution. To fork and link to your own version of Rails just to get something like Array#sum is not sustainable. Yes, Bundler makes those first steps easy, but maintenance doesn't seem particularly fun after that (especially for a project as active as Rails). Pull requests, if done right, are the real way to go. If the pull request isn't accepted (but the maintainer is an active one), maybe you should ask yourself why you need to implement this change in the first place. (Or why you need to implement it this way.) That said, refinements (aka, monkey-patching without consequences) should show up soon and can solve this problem for anyone who upgrades to Ruby 2.
cldwalker 16y ago
The title of this post is misleading. This isn't about ending monkeypatching. This is about putting your monkeypatches in a fork.

28 points by pwim 16y ago

It misses the most important step: submit a pull request to the project maintainer, so that you don't end up maintaining every project yourself for which you ever fixed a bug.

keyle 16y ago
I see so many pull requests that get no feedback and being totally ignored, it gets me wondering about this whole forking fanzy... For major projects it may work, but sometimes you're updating an old library to work with a newer version of a platform and get dead silence. It doesn't do anyone any good.
- swaits 16y ago
  It's just laziness. I recently sent the ditz maintainers a patchset for 1.9 compatibility, which they promptly ignored. They aren't on github/bitbucket, so I had to manually diff a copy of the original code against mine in order to create the patch. Then I needed to subscribe to the mailing list and send them the patch. On their end, they would need to run patch to apply my changes. None of this is hard, we've been doing it for a long time. But at least with github/bitbucket, most of this friction is reduced to a few simple clicks. It's certainly no worse.
  - Someone 16y ago
    It's just laziness. [...] Then I needed to subscribe to the mailing list and send them the patch. On their end, they would need to run patch to apply my changes. If only things were that simple. A proper maintainer would have to: a) decide whether what this patch claims to do with the code moves the code in a direction where the maintainers want it to move. b) review the code for quality, security concerns and outright backdoors. c) possibly: compare it to a zillion patches that (more or less) claim to do the same. d) assuming the patch is perfect: check with collisions on other pending patches, recently committed changes, etc. I do not know about this project, and have no real expertise as a maintainer, but I am fairly sure it is not "just laziness". If it were, all projects would have an email address that blindly applies each patch sent to it to the code base.
    - swaits 16y ago
      > I do not know about this project, and have no real expertise as a maintainer In my example case, it certainly is. There certainly aren't zillions of patches. The maintainers are just not "maintaining". Plain and simple.
- zacharyz 16y ago
  The worst is when you find a project that isn't being maintained but still solves a problem. Then you have to trace through all the different pull requests to find what actually works and is relavent to your needs.

yxhuvud 16y ago

Yes, instead of getting support hell for monkeypatching, we get support hell for using forked versions of the libs. Consider me skeptical that there will be any real improvement from this. (I am not claiming forks are bad, only that it solves a different problem than support hell)

michaelfeathers 16y ago
Agreed. There should be a term for "takes problem from one place and puts it in another place yielding no net advantage."
- JonnieCache 16y ago
  "Shuffling the deckchairs on the titanic" comes to mind, but it isn't really a term.
  - IsaacL 16y ago
    I've heard that phrase a few times before, but I don't think it fits the context here. The context is a "solution" that creates more problems than it solves. "Shuffling the deckchairs/polishing the silverware on the Titanic" refers to busying oneself with small problems when there's a honking great problem that's being ignored.
- riffraff 16y ago
  parasolving? Though this is not really the case, I'd say it's more like "the problem is still there but we walked around it with a different route". Perisolving maybe? Especially if you consider that part of the problem is that something won't work but it may take hours to understand it's because the semantics of Enumerable#group_by in library X are not the same as the ones defined in library Y.
- goodside 16y ago
  Circumsolution.
- Confusion 16y ago

deleted 16y ago

[deleted]

grammaton 16y ago

Monkey patching is bad software engineering, but it happens because it also works in many cases, and because it's fast. The fork and fix approach is a pretty heavy weight solution to an often trivial problem. Just because git (and lets face it, ruby and git go hand in hand) lets you branch to your heart's content, doesn't mean you should.

mckoss 16y ago

The key benefit can be had with monkey patching as well: - Publishing a new solution and making it available. The packaging is not ideal, as setup and configuration of the lib+patch will not be as simple as pulling a modified lib. But think what you would have to do to incorporate 5 separate patches to a given library. It could very well be simpler to run 5 monkey patches than to fork a project and do 5 merges.

bgentry 16y ago

I wish there was more of an emphasis on this being a temporary solution. Usually when you need a patch, you need it now, so pulling the lib from your own git fork is the only immediate solution. But you should make every effort with your fork to adhere to the project's standards and get your fixes merged back in so the entire ecosystem can benefit. Get off the custom gem as soon as possible.

MichaelGG 16y ago

Non-Ruby user here. I thought monkeypatching was replacing members at runtime, not just adding them. Or adding them to an instance of the object instead of the class. Isn't just adding members like a "type extension" that statically typed languages can safely do? (Not flaming, just trying to understand the differences.)

awj 16y ago
That actually is (sort of) addressed in the article's initial example. Your class extension can turn someone else's class extension into a monkey patch. Worse, your extension may not match calling semantics of the other extension, so you can get some truly ugly behavior.

djacobs 16y ago

I really don't like this solution. To fork and link to your own version of Rails just to get something like Array#sum is not sustainable. Yes, Bundler makes those first steps easy, but maintenance doesn't seem particularly fun after that (especially for a project as active as Rails). Pull requests, if done right, are the real way to go. If the pull request isn't accepted (but the maintainer is an active one), maybe you should ask yourself why you need to implement this change in the first place. (Or why you need to implement it this way.) That said, refinements (aka, monkey-patching without consequences) should show up soon and can solve this problem for anyone who upgrades to Ruby 2.

cldwalker 16y ago

The title of this post is misleading. This isn't about ending monkeypatching. This is about putting your monkeypatches in a fork.