D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Ilya Bizyaev
IlyaBizyaev added a comment. In D28444#638430 , @cblack wrote: > more than half of Ikona's CMakeLists.txt is boilerplate dedicated to integrating Rust with the rest of the project I don't understand why your function needs a list of Rust

D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Carson Black
cblack added a comment. In D28444#638435 , @apol wrote: > Will this make all the boilerplate in ikona disappear? It'll bring it down to the same level as everything else. REPOSITORY R240 Extra CMake Modules REVISION DETAIL

D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Aleix Pol Gonzalez
apol added a comment. > I feel like one of the reasons KDE doesn't use Rust more is that using it in the build system side is pain. Case in point: more than half of Ikona's CMakeLists.txt is boilerplate dedicated to integrating Rust with the rest of the project. I'd like to get this in ECM

D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Carson Black
cblack added a comment. In D28444#638428 , @apol wrote: > I wonder if this is covering a very widespread use-case. If there's just one application that needs it, it could make sense to keep it there and once it's polished, we put it in ECM to

D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Aleix Pol Gonzalez
apol added a comment. Code per se looks okay, not that I've ever used a rust workspace. It does need documentation though. See other ECM modules to see what it looks like. I wonder if this is covering a very widespread use-case. If there's just one application that needs it, it

D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Ilya Bizyaev
IlyaBizyaev added a comment. I'm not good at CMake modules, but maybe this code (which I used in a project) is useful: https://github.com/Devolutions/CMakeRust REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D28444 To: cblack, #frameworks,

D28355: Introduce function ecm_install_configured_file

2020-03-30 Thread Friedrich W. H. Kossebau
kossebau added inline comments. INLINE COMMENTS > kossebau wrote in ECMConfiguredInstall.cmake:62 > These strings (besides the last obviously) should get added with whitespace > suffix, to handle the case where multiple are added, no? Not yet got to > test/run things, just guessing by reading

D28355: Introduce function ecm_install_configured_file

2020-03-30 Thread Friedrich W. H. Kossebau
kossebau added a comment. Quick review while I had some spare minutes, to keep things going. INLINE COMMENTS > ECMConfiguredInstall.cmake:5 > +# > +# Take a list of files, runs configure_file and installs it in the given > location. > +# Perhaps "install" -> "install the result" >

D28444: WIP/RFC: Add ECMCargo module

2020-03-30 Thread Carson Black
cblack created this revision. cblack added reviewers: Frameworks, Build System. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. cblack requested review of this revision. REVISION SUMMARY The ECMCargo module allows for easy usage

D28355: Introduce function ecm_install_configured_file

2020-03-30 Thread David Edmundson
davidedmundson updated this revision to Diff 78887. davidedmundson added a comment. All of the things! Only strips suffix ".in". REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28355?vs=78693=78887 BRANCH master REVISION DETAIL

D28355: Introduce function ecm_install_configured_file

2020-03-30 Thread David Edmundson
davidedmundson added a comment. I agree with almost all those suggestions. Though it's a lot more complex, so I might need some help with some of that. As for intended usages, it came up on: D28305 for a lot of .service files with a binary location