You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/09/04 18:01:35 UTC
Review Request 62071: Added non-src files to CMake project to
facilitate indexing.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62071/
-----------------------------------------------------------
Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
Repository: mesos
Description
-------
Some IDEs and indexing tools build the project structure from CMake
target definitions. In this case files that are not essential for
building the target and are omitted from the target declaration,
do not appear in the project either and hence are not searchable
inside the IDE or indexing tool.
Diffs
-----
CMakeLists.txt e08fcef746460b601c1bddf9e0f00cad31500ec9
src/CMakeLists.txt 0562b2bf87fb6e2b65a2a512e694e89ee431738b
Diff: https://reviews.apache.org/r/62071/diff/1/
Testing
-------
Run CMake and ensured configure + generate passes;
Run Qt Creator with the built-in CMake plugin and verified the change adds files to the project structure and makes them searchable in Qt Creator.
Thanks,
Alexander Rukletsov
Re: Review Request 62071: Added non-src files to CMake project to
facilitate indexing.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62071/#review184553
-----------------------------------------------------------
FAIL: 'Mesos testes failed to run'
Reviews applied: [62070, 62071]
Logs available here: 'http://mesos-logs.westus.cloudapp.azure.com/mesos-build/review/61211/logs'
- Mesos Reviewbot Windows
On Sept. 4, 2017, 6:01 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62071/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2017, 6:01 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Some IDEs and indexing tools build the project structure from CMake
> target definitions. In this case files that are not essential for
> building the target and are omitted from the target declaration,
> do not appear in the project either and hence are not searchable
> inside the IDE or indexing tool.
>
>
> Diffs
> -----
>
> CMakeLists.txt e08fcef746460b601c1bddf9e0f00cad31500ec9
> src/CMakeLists.txt 0562b2bf87fb6e2b65a2a512e694e89ee431738b
>
>
> Diff: https://reviews.apache.org/r/62071/diff/1/
>
>
> Testing
> -------
>
> Run CMake and ensured configure + generate passes;
> Run Qt Creator with the built-in CMake plugin and verified the change adds files to the project structure and makes them searchable in Qt Creator.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 62071: Added non-src files to CMake project to
facilitate indexing.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62071/#review184724
-----------------------------------------------------------
src/CMakeLists.txt
Line 597 (original), 597 (patched)
<https://reviews.apache.org/r/62071/#comment260914>
I'm not a huge fan of this approach: adding these as explicit dependencies to the library. I understand that it works for the intended change (and quite simply I might add); however, I fear it means that a change to e.g. `CHANGELOG` or any of the support scripts/docs etc. would trigger a rebuild of the library. We need to be very careful about not breaking incremental builds.
I mentioned in the prior review maybe looking at `source_group()` again. I'm fine with globbing, but I know some maintainers aren't a big fan.
NOTE: I could be super wrong, as I haven't tested this. I would totally believe CMake is smart enough not to add these as dependencies in such a manner as to break incremental builds. I'd like us to be certain we're all happy with the approach we take.
- Andrew Schwartzmeyer
On Sept. 4, 2017, 11:01 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62071/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2017, 11:01 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Some IDEs and indexing tools build the project structure from CMake
> target definitions. In this case files that are not essential for
> building the target and are omitted from the target declaration,
> do not appear in the project either and hence are not searchable
> inside the IDE or indexing tool.
>
>
> Diffs
> -----
>
> CMakeLists.txt e08fcef746460b601c1bddf9e0f00cad31500ec9
> src/CMakeLists.txt 0562b2bf87fb6e2b65a2a512e694e89ee431738b
>
>
> Diff: https://reviews.apache.org/r/62071/diff/1/
>
>
> Testing
> -------
>
> Run CMake and ensured configure + generate passes;
> Run Qt Creator with the built-in CMake plugin and verified the change adds files to the project structure and makes them searchable in Qt Creator.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>