mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 44288: Changed MasterDetector/Contender namespace.
Date Thu, 03 Mar 2016 22:15:33 GMT

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



Partial review to check for non-namespace-changes:


include/mesos/scheduler.hpp (lines 44 - 49)
<https://reviews.apache.org/r/44288/#comment183790>

    A forward declaration isn't required anymore.  You can just include the header(s) from
the previous review.



include/mesos/v1/scheduler.hpp (lines 34 - 38)
<https://reviews.apache.org/r/44288/#comment183791>

    Ditto.



src/cli/resolve.cpp (line 29)
<https://reviews.apache.org/r/44288/#comment183792>

    Nit: Mesos includes go above libprocess includes (`<process/...>`).



src/master/contender.hpp (lines 17 - 18)
<https://reviews.apache.org/r/44288/#comment183793>

    This whole file seems like a pretty substantial change.  I'd recommend pulling it out
into a separate review (rather than hiding it in this review).
    
    Also, you'll want to consider making folders "contenders" and "detectors".  Then renaming
this file "standalone.hpp".



src/master/contender.cpp (lines 25 - 26)
<https://reviews.apache.org/r/44288/#comment183795>

    Ditto with this file.



src/master/detector.hpp (lines 17 - 18)
<https://reviews.apache.org/r/44288/#comment183794>

    Ditto with this file.



src/master/detector.cpp (lines 33 - 34)
<https://reviews.apache.org/r/44288/#comment183796>

    Ditto with this file.



src/master/main.cpp (line 60)
<https://reviews.apache.org/r/44288/#comment183797>

    What about this header?



src/master/main.cpp (lines 154 - 173)
<https://reviews.apache.org/r/44288/#comment183789>

    These flags should really go into `src/master/flags.cpp`.  And you shouldn't tuck extra
changes into a big "renaming" review (pull out into a separate review).
    
    Same for the rest of the changes in this file.



src/master/master.hpp (lines 72 - 73)
<https://reviews.apache.org/r/44288/#comment183798>

    What about these?


- Joseph Wu


On March 3, 2016, 9:30 a.m., Anurag Singh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44288/
> -----------------------------------------------------------
> 
> (Updated March 3, 2016, 9:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4610
>     https://issues.apache.org/jira/browse/MESOS-4610
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Also modified users of MasterContender and MasterDetector to use this
> namespace.
> 
> 
> Diffs
> -----
> 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
>   src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 
>   src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
>   src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
>   src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
>   src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
>   src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/master/master.cpp f242b70a0ebeff9cd3204343735b8615230a6108 
>   src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
>   src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 
>   src/tests/fault_tolerance_tests.cpp f2b8dba809518cf716b2b5a7a6a8a5fe62e57646 
>   src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 29c89fb11da792c3e71eb880a19657ea225b3cc8 
>   src/tests/master_contender_detector_tests.cpp 255ab8119a04b55bb4f1b61dee19c4be64499376

>   src/tests/master_slave_reconciliation_tests.cpp d41178eb41df519073fc0890c5716bbc9fed6ad2

>   src/tests/master_tests.cpp 36595772b34bcb8d37dbc74d247bdf4614f10150 
>   src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_tests.cpp bf19c81fbcf973d1ac27fbd42eedfd7118b7ba50 
>   src/tests/reconciliation_tests.cpp 97112c4d64c75a16fdd7bbefd517a039fbf55b64 
>   src/tests/reservation_tests.cpp d7f9de6f2bce061316916260f356efdb96ecd482 
>   src/tests/scheduler_event_call_tests.cpp 8c02ceeb3ec1783cb2f63f100700508e70f586e4 
>   src/tests/scheduler_http_api_tests.cpp 428e12646d80b45daec30cfe607b97f36170fdf5 
>   src/tests/scheduler_tests.cpp 2b1693eaf1a6106f5e7d269e4e3f6c353dd6f017 
>   src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
>   src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 
> 
> Diff: https://reviews.apache.org/r/44288/diff/
> 
> 
> Testing
> -------
> 
> See https://reviews.apache.org/r/44289/.
> 
> 
> Thanks,
> 
> Anurag Singh
> 
>


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