You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jeff Coffler <je...@taltos.com> on 2017/10/11 23:29:55 UTC

Re: Review Request 60620: Modifed os::write to write binary files on Windows.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/
-----------------------------------------------------------

(Updated Oct. 11, 2017, 11:29 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.


Summary (updated)
-----------------

Modifed os::write to write binary files on Windows.


Bugs: MESOS-6705
    https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description (updated)
-------

Modifed os::write to write binary files on Windows.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 


Diff: https://reviews.apache.org/r/60620/diff/4/

Changes: https://reviews.apache.org/r/60620/diff/3-4/


Testing
-------

Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Jeff Coffler <je...@taltos.com>.

> On Oct. 13, 2017, 8:15 p.m., Andrew Schwartzmeyer wrote:
> > This still needs a description (currently it's just a copy of the summary).

I didn't realize that commits needed a description. I thought that if it was obvious enough, a detailed description was not needed. Indeed, I see a bunch of commits to Mesos with no description (just the one line summary).

In any case, I'll be adding descriptions to all commits in this chain, and in the future.


- Jeff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/#review188007
-----------------------------------------------------------


On Oct. 11, 2017, 11:29 p.m., Jeff Coffler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60620/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 11:29 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
>     https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Modifed os::write to write binary files on Windows.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 
> 
> 
> Diff: https://reviews.apache.org/r/60620/diff/4/
> 
> 
> Testing
> -------
> 
> Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).
> 
> Ran stout-tests and mesos-tests successfully on both Windows and Linux.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.

> On Oct. 13, 2017, 1:15 p.m., Andrew Schwartzmeyer wrote:
> > This still needs a description (currently it's just a copy of the summary).
> 
> Jeff Coffler wrote:
>     I didn't realize that commits needed a description. I thought that if it was obvious enough, a detailed description was not needed. Indeed, I see a bunch of commits to Mesos with no description (just the one line summary).
>     
>     In any case, I'll be adding descriptions to all commits in this chain, and in the future.

In general they should have a description, especially to avoid the problem with ReviewBoard copying the summary to the description (and requiring committers to manually fix it as they apply patches).


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/#review188007
-----------------------------------------------------------


On Oct. 11, 2017, 4:29 p.m., Jeff Coffler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60620/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 4:29 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
>     https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Modifed os::write to write binary files on Windows.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 
> 
> 
> Diff: https://reviews.apache.org/r/60620/diff/4/
> 
> 
> Testing
> -------
> 
> Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).
> 
> Ran stout-tests and mesos-tests successfully on both Windows and Linux.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/#review188007
-----------------------------------------------------------



This still needs a description (currently it's just a copy of the summary).

- Andrew Schwartzmeyer


On Oct. 11, 2017, 4:29 p.m., Jeff Coffler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60620/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2017, 4:29 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
>     https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Modifed os::write to write binary files on Windows.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 
> 
> 
> Diff: https://reviews.apache.org/r/60620/diff/4/
> 
> 
> Testing
> -------
> 
> Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).
> 
> Ran stout-tests and mesos-tests successfully on both Windows and Linux.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Jeff Coffler <je...@taltos.com>.

> On Oct. 17, 2017, 11:12 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/os/write.hpp
> > Lines 107 (patched)
> > <https://reviews.apache.org/r/60620/diff/5/?file=1858851#file1858851line107>
> >
> >     nit: extra whitespace introduced

Removed blank line.


- Jeff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/#review188412
-----------------------------------------------------------


On Oct. 17, 2017, 1:16 a.m., Jeff Coffler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60620/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2017, 1:16 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
>     https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By default, ::write operations on Windows will include a \r\n (CR/LF)
> at the end of each line, which is not compatible with Linux. Mesos
> expects only \n (LF) characters between lines.
> 
> This commit will modify Windows behavior to only include \n at the
> end of each line written to a text file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 
> 
> 
> Diff: https://reviews.apache.org/r/60620/diff/5/
> 
> 
> Testing
> -------
> 
> Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).
> 
> Ran stout-tests and mesos-tests successfully on both Windows and Linux.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/#review188412
-----------------------------------------------------------




3rdparty/stout/include/stout/os/write.hpp
Lines 107 (patched)
<https://reviews.apache.org/r/60620/#comment265405>

    nit: extra whitespace introduced


- Andrew Schwartzmeyer


On Oct. 16, 2017, 6:16 p.m., Jeff Coffler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60620/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 6:16 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
>     https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> By default, ::write operations on Windows will include a \r\n (CR/LF)
> at the end of each line, which is not compatible with Linux. Mesos
> expects only \n (LF) characters between lines.
> 
> This commit will modify Windows behavior to only include \n at the
> end of each line written to a text file.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 
> 
> 
> Diff: https://reviews.apache.org/r/60620/diff/5/
> 
> 
> Testing
> -------
> 
> Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).
> 
> Ran stout-tests and mesos-tests successfully on both Windows and Linux.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/
-----------------------------------------------------------

(Updated Oct. 19, 2017, 9:47 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.


Bugs: MESOS-6705
    https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
-------

By default, ::write operations on Windows will include a \r\n (CR/LF)
at the end of each line, which is not compatible with Linux. Mesos
expects only \n (LF) characters between lines.

This commit will modify Windows behavior to only include \n at the
end of each line written to a text file.


Diffs
-----

  3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 


Diff: https://reviews.apache.org/r/60620/diff/7/


Testing
-------

Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/
-----------------------------------------------------------

(Updated Oct. 19, 2017, 9:34 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.


Bugs: MESOS-6705
    https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
-------

By default, ::write operations on Windows will include a \r\n (CR/LF)
at the end of each line, which is not compatible with Linux. Mesos
expects only \n (LF) characters between lines.

This commit will modify Windows behavior to only include \n at the
end of each line written to a text file.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 


Diff: https://reviews.apache.org/r/60620/diff/7/

Changes: https://reviews.apache.org/r/60620/diff/6-7/


Testing
-------

Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/
-----------------------------------------------------------

(Updated Oct. 19, 2017, 6:15 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.


Bugs: MESOS-6705
    https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description
-------

By default, ::write operations on Windows will include a \r\n (CR/LF)
at the end of each line, which is not compatible with Linux. Mesos
expects only \n (LF) characters between lines.

This commit will modify Windows behavior to only include \n at the
end of each line written to a text file.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 


Diff: https://reviews.apache.org/r/60620/diff/6/

Changes: https://reviews.apache.org/r/60620/diff/5-6/


Testing
-------

Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler


Re: Review Request 60620: Modifed os::write to write binary files on Windows.

Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60620/
-----------------------------------------------------------

(Updated Oct. 17, 2017, 1:16 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.


Bugs: MESOS-6705
    https://issues.apache.org/jira/browse/MESOS-6705


Repository: mesos


Description (updated)
-------

By default, ::write operations on Windows will include a \r\n (CR/LF)
at the end of each line, which is not compatible with Linux. Mesos
expects only \n (LF) characters between lines.

This commit will modify Windows behavior to only include \n at the
end of each line written to a text file.


Diffs (updated)
-----

  3rdparty/stout/include/stout/os/write.hpp 9ff749f209e6dd6ca3695907108a029c9a2b4f05 


Diff: https://reviews.apache.org/r/60620/diff/5/

Changes: https://reviews.apache.org/r/60620/diff/4-5/


Testing
-------

Built successfully on both Linux (with autotools and cmake) and Windows (with cmake).

Ran stout-tests and mesos-tests successfully on both Windows and Linux.


Thanks,

Jeff Coffler