You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2016/12/20 13:53:25 UTC
Review Request 54896: Fixed copy-template-and-create-symlink make
target.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54896/
-----------------------------------------------------------
Review request for mesos, James Peach and Till Toenshoff.
Repository: mesos
Description
-------
This target was `PHONY`, but not declared as such. It also explicitly
used `ln` with a `--force` flag instead of the more generally
applicable and suggested `$(LN_S)` we use elsewhere.
This change fixes above issues. We also use independent shell commands
for independent commands and fix some whitespace issues.
Diffs
-----
src/Makefile.am 0f62ec70816e8b48e19d35036285656a6e7cd02b
Diff: https://reviews.apache.org/r/54896/diff/
Testing
-------
`make distcheck` (Fedora 25)
Thanks,
Benjamin Bannier
Re: Review Request 54896: Fixed copy-template-and-create-symlink make
target.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Dec. 20, 2016, 9:08 p.m., James Peach wrote:
> > src/Makefile.am, line 2428
> > <https://reviews.apache.org/r/54896/diff/1/?file=1589696#file1589696line2428>
> >
> > Should this be a symlink too? Making a copy creates the possibility that you could add environment variables in one file but not the other?
Good idea. I am less worried about edits to this file since it is just a template, but more about this almost suggesting that "agent" and "slave" are different entities.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54896/#review159743
-----------------------------------------------------------
On Dec. 20, 2016, 2:53 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54896/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2016, 2:53 p.m.)
>
>
> Review request for mesos, James Peach and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This target was `PHONY`, but not declared as such. It also explicitly
> used `ln` with a `--force` flag instead of the more generally
> applicable and suggested `$(LN_S)` we use elsewhere.
>
> This change fixes above issues. We also use independent shell commands
> for independent commands and fix some whitespace issues.
>
>
> Diffs
> -----
>
> src/Makefile.am 0f62ec70816e8b48e19d35036285656a6e7cd02b
>
> Diff: https://reviews.apache.org/r/54896/diff/
>
>
> Testing
> -------
>
> `make distcheck` (Fedora 25)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 54896: Fixed copy-template-and-create-symlink make
target.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54896/#review159743
-----------------------------------------------------------
src/Makefile.am (line 2428)
<https://reviews.apache.org/r/54896/#comment230771>
Should this be a symlink too? Making a copy creates the possibility that you could add environment variables in one file but not the other?
- James Peach
On Dec. 20, 2016, 1:53 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54896/
> -----------------------------------------------------------
>
> (Updated Dec. 20, 2016, 1:53 p.m.)
>
>
> Review request for mesos, James Peach and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This target was `PHONY`, but not declared as such. It also explicitly
> used `ln` with a `--force` flag instead of the more generally
> applicable and suggested `$(LN_S)` we use elsewhere.
>
> This change fixes above issues. We also use independent shell commands
> for independent commands and fix some whitespace issues.
>
>
> Diffs
> -----
>
> src/Makefile.am 0f62ec70816e8b48e19d35036285656a6e7cd02b
>
> Diff: https://reviews.apache.org/r/54896/diff/
>
>
> Testing
> -------
>
> `make distcheck` (Fedora 25)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 54896: Fixed copy-template-and-create-symlink make
target.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54896/#review159839
-----------------------------------------------------------
Ship it!
Ship It!
- James Peach
On Dec. 21, 2016, 11:20 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54896/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2016, 11:20 a.m.)
>
>
> Review request for mesos, James Peach and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This target was `PHONY`, but not declared as such. It also explicitly
> used `ln` with a `--force` flag instead of the more generally
> applicable and suggested `$(LN_S)` we use elsewhere, and was
> inconsistent on whether content was linked or copied.
>
> This change fixes above issues. We also use independent shell commands
> for independent commands and fix some whitespace issues.
>
>
> Diffs
> -----
>
> src/Makefile.am abcf7eed717ccd2396540011d7fb9fc7959c18f2
>
> Diff: https://reviews.apache.org/r/54896/diff/
>
>
> Testing
> -------
>
> `make distcheck` (Fedora 25)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 54896: Fixed copy-template-and-create-symlink make
target.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54896/#review159943
-----------------------------------------------------------
Ship it!
Ship It!
- Till Toenshoff
On Dec. 21, 2016, 11:20 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54896/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2016, 11:20 a.m.)
>
>
> Review request for mesos, James Peach and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This target was `PHONY`, but not declared as such. It also explicitly
> used `ln` with a `--force` flag instead of the more generally
> applicable and suggested `$(LN_S)` we use elsewhere, and was
> inconsistent on whether content was linked or copied.
>
> This change fixes above issues. We also use independent shell commands
> for independent commands and fix some whitespace issues.
>
>
> Diffs
> -----
>
> src/Makefile.am abcf7eed717ccd2396540011d7fb9fc7959c18f2
>
> Diff: https://reviews.apache.org/r/54896/diff/
>
>
> Testing
> -------
>
> `make distcheck` (Fedora 25)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 54896: Fixed copy-template-and-create-symlink make
target.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54896/
-----------------------------------------------------------
(Updated Dec. 21, 2016, 12:20 p.m.)
Review request for mesos, James Peach and Till Toenshoff.
Repository: mesos
Description (updated)
-------
This target was `PHONY`, but not declared as such. It also explicitly
used `ln` with a `--force` flag instead of the more generally
applicable and suggested `$(LN_S)` we use elsewhere, and was
inconsistent on whether content was linked or copied.
This change fixes above issues. We also use independent shell commands
for independent commands and fix some whitespace issues.
Diffs (updated)
-----
src/Makefile.am abcf7eed717ccd2396540011d7fb9fc7959c18f2
Diff: https://reviews.apache.org/r/54896/diff/
Testing
-------
`make distcheck` (Fedora 25)
Thanks,
Benjamin Bannier