You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2020/02/25 01:07:47 UTC
Review Request 72161: Added patch for RapidJSON.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik and Greg Mann.
Bugs: MESOS-10064
https://issues.apache.org/jira/browse/MESOS-10064
Repository: mesos
Description
-------
This commit updates the writer of RapidJSON to write infinite
floating point numbers as "Infinity" and "-Infinity" (i.e.,
with double quotes) rather than Infinity and -Infinity. This
is to ensure the strings converted from JSON objects conform
to the rule defined by Protobuf:
https://developers.google.com/protocol-buffers/docs/proto3#json
Diffs
-----
3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
3rdparty/rapidjson-1.1.0.patch PRE-CREATION
Diff: https://reviews.apache.org/r/72161/diff/1/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Qian Zhang <zh...@gmail.com>.
> On Feb. 28, 2020, 12:35 a.m., Greg Mann wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 482-484 (original), 482-487 (patched)
> > <https://reviews.apache.org/r/72161/diff/1/?file=2212006#file2212006line482>
> >
> > We need to update the autotools build as well right?
It is already handled here: https://github.com/apache/mesos/blob/1.9.0/3rdparty/Makefile.am#L168:L172
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review219676
-----------------------------------------------------------
On Feb. 25, 2020, 9:07 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2020, 9:07 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review219676
-----------------------------------------------------------
3rdparty/CMakeLists.txt
Lines 482-484 (original), 482-487 (patched)
<https://reviews.apache.org/r/72161/#comment307889>
We need to update the autotools build as well right?
- Greg Mann
On Feb. 25, 2020, 1:07 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2020, 1:07 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/
-----------------------------------------------------------
(Updated March 20, 2020, 8:53 a.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Addressed review comment.
Bugs: MESOS-10064
https://issues.apache.org/jira/browse/MESOS-10064
Repository: mesos
Description
-------
This commit updates the writer of RapidJSON to write infinite
floating point numbers as "Infinity" and "-Infinity" (i.e.,
with double quotes) rather than Infinity and -Infinity. This
is to ensure the strings converted from JSON objects conform
to the rule defined by Protobuf:
https://developers.google.com/protocol-buffers/docs/proto3#json
Diffs (updated)
-----
3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
3rdparty/Makefile.am 243a6190d436a4ec179aaaf37e1c6284f35917c7
3rdparty/rapidjson-1.1.0.patch PRE-CREATION
Diff: https://reviews.apache.org/r/72161/diff/4/
Changes: https://reviews.apache.org/r/72161/diff/3-4/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review220022
-----------------------------------------------------------
3rdparty/CMakeLists.txt
Line 477 (original), 477 (patched)
<https://reviews.apache.org/r/72161/#comment308284>
I think we also need to update '3rdparty/Makefile.am' to add this patch file to EXTRA_DIST.
- Greg Mann
On March 16, 2020, 9:03 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated March 16, 2020, 9:03 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/
-----------------------------------------------------------
(Updated March 16, 2020, 5:03 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Rebased.
Bugs: MESOS-10064
https://issues.apache.org/jira/browse/MESOS-10064
Repository: mesos
Description
-------
This commit updates the writer of RapidJSON to write infinite
floating point numbers as "Infinity" and "-Infinity" (i.e.,
with double quotes) rather than Infinity and -Infinity. This
is to ensure the strings converted from JSON objects conform
to the rule defined by Protobuf:
https://developers.google.com/protocol-buffers/docs/proto3#json
Diffs (updated)
-----
3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
3rdparty/rapidjson-1.1.0.patch PRE-CREATION
Diff: https://reviews.apache.org/r/72161/diff/3/
Changes: https://reviews.apache.org/r/72161/diff/2-3/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review219769
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On March 2, 2020, 6:27 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated March 2, 2020, 6:27 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/
-----------------------------------------------------------
(Updated March 2, 2020, 2:27 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Patched `bool Writer<StringBuffer>::WriteDouble(double d)` as well.
Bugs: MESOS-10064
https://issues.apache.org/jira/browse/MESOS-10064
Repository: mesos
Description
-------
This commit updates the writer of RapidJSON to write infinite
floating point numbers as "Infinity" and "-Infinity" (i.e.,
with double quotes) rather than Infinity and -Infinity. This
is to ensure the strings converted from JSON objects conform
to the rule defined by Protobuf:
https://developers.google.com/protocol-buffers/docs/proto3#json
Diffs (updated)
-----
3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
3rdparty/rapidjson-1.1.0.patch PRE-CREATION
Diff: https://reviews.apache.org/r/72161/diff/2/
Changes: https://reviews.apache.org/r/72161/diff/1-2/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Feb. 27, 2020, 4:59 p.m., Benjamin Bannier wrote:
> > Is the plan to upstream this patch? We usually try to enable building against unbundled dependencies and depending on custom behavior makes that impossible. My hunch would be to not make functional changes like the one in this patch to dependencies (changes for building are usually fine).
Yea, attempting to upstream is a good idea. Qian would you like to open a PR, or would you like me to? https://github.com/Tencent/rapidjson
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review219677
-----------------------------------------------------------
On March 2, 2020, 6:27 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated March 2, 2020, 6:27 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Qian Zhang <zh...@gmail.com>.
> On Feb. 28, 2020, 12:59 a.m., Benjamin Bannier wrote:
> > Is the plan to upstream this patch? We usually try to enable building against unbundled dependencies and depending on custom behavior makes that impossible. My hunch would be to not make functional changes like the one in this patch to dependencies (changes for building are usually fine).
>
> Greg Mann wrote:
> Yea, attempting to upstream is a good idea. Qian would you like to open a PR, or would you like me to? https://github.com/Tencent/rapidjson
I am not sure if it is something which will be accepted by upstream, what I did in this patch is to make RapidJSON confirm to the rule defined by ProtoBuf (https://developers.google.com/protocol-buffers/docs/proto3#json), I am not sure if that is a general rule that RapidJSON will care about.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review219677
-----------------------------------------------------------
On March 2, 2020, 2:27 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated March 2, 2020, 2:27 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72161: Added patch for RapidJSON.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72161/#review219677
-----------------------------------------------------------
Is the plan to upstream this patch? We usually try to enable building against unbundled dependencies and depending on custom behavior makes that impossible. My hunch would be to not make functional changes like the one in this patch to dependencies (changes for building are usually fine).
- Benjamin Bannier
On Feb. 25, 2020, 2:07 vorm., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> -----------------------------------------------------------
>
> (Updated Feb. 25, 2020, 2:07 vorm.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
>
>
> Diffs
> -----
>
> 3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd
> 3rdparty/rapidjson-1.1.0.patch PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/72161/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>