mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bernd Mathiske" <be...@mesosphere.io>
Subject Re: Review Request 41939: Simplified quota tests.
Date Mon, 11 Jan 2016 13:01:43 GMT

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



src/tests/master_quota_tests.cpp (line 376)
<https://reviews.apache.org/r/41939/#comment174560>

    Please connect this comment explicitly to the occurrence of "true" that is hinted at here.
Suggestion:
    
    Put this constant in a central location:
        // Use the force flag for setting quota that cannot be satisfied
        // in this empty cluster without any agents.
        const bool FORCE = true;
        
    Then use the constant like this:
        Future<Response> response = process::http::post(
            master.get(),
            "quota",
            createBasicAuthHeaders(DEFAULT_CREDENTIAL),
            createRequestBody(ROLE1, quotaResources, FORCE));
            
    Same for all other mentions of the force flag in this patch.
    
    (In general, I'd suggest it would be better not to use bool parameters for anything in
the first place.)


- Bernd Mathiske


On Jan. 5, 2016, 3:39 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41939/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 3:39 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> For clarity and in order not to distract a reader with copy-paste code:
>   - some tests were merged together;
>   - removed creation of agents where possible and used force flag
>     in quota request instead.
> 
> 
> Diffs
> -----
> 
>   src/tests/master_quota_tests.cpp 2f1bc3ae6a370e466f7cea9b597f51d7eccb1b33 
> 
> Diff: https://reviews.apache.org/r/41939/diff/
> 
> 
> Testing
> -------
> 
> `make check` on Mac OS 10.10.5
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


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