mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 50957: Added fetch method based on plugin name.
Date Tue, 06 Sep 2016 02:39:30 GMT

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


Fix it, then Ship it!




Did some tweak while commiting. Please take a look at the final patch.


include/mesos/uri/fetcher.hpp (line 81)
<https://reviews.apache.org/r/50957/#comment215029>

    Since each plugins has scheme() and name(), i'd suggest we take a vector<Owned<Plugin>>
here and construct the hashmaps in the contructor.
    
    Also, it does not make sense to use Owned pointer given that two hashmaps will store the
same Owned object. We should use Shared here. That means Plugin::fetch needs to be const (which
is OK).



include/mesos/uri/fetcher.hpp (line 115)
<https://reviews.apache.org/r/50957/#comment215064>

    s/plugins/pluginsByScheme/


- Jie Yu


On Aug. 25, 2016, 3:22 p.m., Srinivas Brahmaroutu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50957/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4288
>     https://issues.apache.org/jira/browse/MESOS-4288
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added plugins by name into the Fetcher. Added a new fetch
> method to select plugin by name which ignores scheme and
> directly invokes fetch on the plugin.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp 3add35c8c0e559203acb540a288d0b51ac817519 
>   src/uri/fetcher.cpp 198bd29993381758183edfce8faafba36da2d9ae 
> 
> Diff: https://reviews.apache.org/r/50957/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message