@b4n requested changes on this pull request.

This has a few issues IMO:
* The way you modify the file *changes the whole line*, meaning that if that 
rule and the source file are not in sync, it's super hard to diagnose weird 
content in the installed file (as it ignores the content of the source file).
* I don't like modifying a file at install time.  Not only this means you get 
another set of files before and after installation, this also means that for a 
lot of people it'll modify build files *as root* (if they install as root), 
which is awfully annoying (you get root-owned files you can't modify in a user 
directory).
* The 
[documentation](https://www.gnu.org/software/automake/manual/automake.html#Extending-Installation)
 doesn't actually say whether or not `install-data-local` is performed before 
or after the Automake rules, so it's not guaranteed modifying a file will 
install it.  In practice it's probably the case if it currently works (and I 
guess the implementation is straightforward and guarantees it), but it's 
undocumented.

I'll abuse of my dictatorship here and say that if we want this we'll use my 
set of changes 
(https://github.com/b4n/geany/commit/884309bd6e7969f7b2d01592fa6741cd76b1580b), 
unless of course anybody has specific concerns about it.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/2728#pullrequestreview-570468082

Reply via email to