> On 八月 16, 2016, 11:44 p.m., Jiang Yan Xu wrote:
> > include/mesos/resources.hpp, line 59
> > <https://reviews.apache.org/r/50836/diff/1/?file=1464015#file1464015line59>
> >
> >     I think it would still work if you keep the `static` keyword right?
> >     
> >     The `friend` keyword wouldn't allow you to specify `static` but here 
> > you can. 
> >     
> >     Then in the cpp files you don't have to remove the `static` keyword on 
> > the definition.

Thanks Jiang Yan, but it still does not work as your proposal by adding 
`static` for the `convertJSON` definition in cpp file and forward declarations 
in hpp file as the `friend Try<Resources> internal::convertJSON()` will still 
think this method is not a `static` method and this will cause build error as 
following.

```
In file included from ../../src/common/resources_utils.cpp:21:
In file included from ../../src/common/resources_utils.hpp:21:
../../include/mesos/resources.hpp:59:25: error: unused function 'convertJSON' 
[-Werror,-Wunused-function]
  static Try<Resources> convertJSON(
                        ^
1 error generated.
make[2]: *** [common/libmesos_no_3rdparty_la-resources_utils.lo] Error 1
```

Another is that I have posted some comments in 
https://reviews.apache.org/r/50568/ and proposing keep `add` as public, can you 
please take a look and show your comments if any.

The following are comments cited from https://reviews.apache.org/r/50568/

```
@Ben and @Yan, I found another place where we can enhance for allocator 
https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.cpp#L1274
 and this was only used by quota now, I think we can use add here. The problem 
is that @Yan porposed to make the add as a private method and I've posted a 
patch here https://reviews.apache.org/r/50836/diff/1#index_header , but after a 
second thought, perhaps we can enable framework developers or other users call 
this add API directly. As sometimes, the user may want to traverse all 
Resources object and make some update for each Resource object and then add 
those Resource again just like what we did in allocator for now. So what about 
keep add as a public method?
```


- Guangya


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


On 八月 5, 2016, 7:31 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50836/
> -----------------------------------------------------------
> 
> (Updated 八月 5, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We do not want people call add/subtract resoure object directly,
> so should make those methods as private.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp bdbe8ea03a50cd1361640bde13a2342909723fc5 
>   include/mesos/v1/resources.hpp c05cb634c7a5add78da00cb84fc75d3472a341bc 
>   src/common/resources.cpp 2470c0280db7d48d9484c42bc2150e53e7ce6e1c 
>   src/v1/resources.cpp 6c4e3b299e701d477947dd7427c31d2ae64c05ae 
> 
> Diff: https://reviews.apache.org/r/50836/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> ```
> ./bin/mesos-tests.sh --gtest_filter="ResourcesTest.*"
> ./bin/mesos-tests.sh --gtest_filter="SharedResourcesTest.*"
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Reply via email to