Patch from a merge request about d9book HOT 4 CLOSED

Sorry, I would do a PR but that would take me quite longer.

I don't think it's enough.

It should clearly state that they should download the MR patch after review, and then have it applied with cweagans/composer-patches with a local reference like: patches/core-1234567-33.patch.

Ideally, this should be done with any patch, but it's really important with MRs.

For MRs: the .patch url thingy will include whatever is on the MR at the request time. If there were any new commits since you reviewed, you are getting those. That's dangerous, and that's what chx was referring to. First, maybe there are commits you didn't test. Second and most important, a malicious user could have committed something with a bad intent, and now you are giving them the keys to your castle voluntarily.

For drupal issue patches: even if not a security issue, your composer install (or deployment) now depends on drupal.org. If you are building your site and drupal.org has a temp outage, you can't complete it. That's undesired.
EDIT: also recently knew that when new d.org is released, issues will be moved to gitlab. Those file urls will stop working at some point, and if they do your project won't build. Be future proof, use local patches.

from d9book.

Yes, this need to be changed to say to download that locally and apply it after careful review.

from d9book.

Thanks folks. I added a big red danger note.

from d9book.

Thanks for the clarification. I updated the description to make those points. btw @penyaskito It is super easy to suggest edits. Just click the Edit this page on Github at the bottom of the page.

from d9book.