You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2016/04/12 02:03:16 UTC

Review Request 46064: Removing ResourceVector enum in favor of ResourceType

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

Review request for Aurora, Bill Farner and Zameer Manji.


Repository: aurora


Description
-------

First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.


Diffs
-----

  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
  src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 

Diff: https://reviews.apache.org/r/46064/diff/


Testing
-------

./gradlew -Pq build


Thanks,

Maxim Khutornenko


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46064/#review128491
-----------------------------------------------------------


Ship it!




Master (a4853a4) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On April 12, 2016, 6:09 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 6:09 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46064/
-----------------------------------------------------------

(Updated April 12, 2016, 6:09 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
-------

Bill's comments.


Repository: aurora


Description
-------

First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.


Diffs (updated)
-----

  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
  src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 

Diff: https://reviews.apache.org/r/46064/diff/


Testing
-------

./gradlew -Pq build


Thanks,

Maxim Khutornenko


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Zameer Manji <zm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46064/#review128479
-----------------------------------------------------------


Ship it!




Ship It!

- Zameer Manji


On April 11, 2016, 5:03 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 5:03 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 12, 2016, 1:39 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java, line 31
> > <https://reviews.apache.org/r/46064/diff/1/?file=1340123#file1340123line31>
> >
> >     It's probably time for some constants to justify these constants (or at least admit that they are lies!).

Not sure I follow, these _are_ the constants baked into the enum with the intention to hide all definitions inside this class and anonymize values via getters. Exposing global constants here will defeat the purpose of this change.


- Maxim


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


On April 12, 2016, 12:03 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 12:03 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Maxim Khutornenko <ma...@apache.org>.

> On April 12, 2016, 1:39 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java, line 31
> > <https://reviews.apache.org/r/46064/diff/1/?file=1340123#file1340123line31>
> >
> >     It's probably time for some constants to justify these constants (or at least admit that they are lies!).
> 
> Maxim Khutornenko wrote:
>     Not sure I follow, these _are_ the constants baked into the enum with the intention to hide all definitions inside this class and anonymize values via getters. Exposing global constants here will defeat the purpose of this change.
> 
> Bill Farner wrote:
>     Ugh, sorry - haste on my part.  *Comments*, not constants :-/

Added constructor docs and linked to getters that already document field specifics.


- Maxim


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


On April 12, 2016, 12:03 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 12:03 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Bill Farner <wf...@apache.org>.

> On April 11, 2016, 6:39 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java, line 31
> > <https://reviews.apache.org/r/46064/diff/1/?file=1340123#file1340123line31>
> >
> >     It's probably time for some constants to justify these constants (or at least admit that they are lies!).
> 
> Maxim Khutornenko wrote:
>     Not sure I follow, these _are_ the constants baked into the enum with the intention to hide all definitions inside this class and anonymize values via getters. Exposing global constants here will defeat the purpose of this change.

Ugh, sorry - haste on my part.  *Comments*, not constants :-/


- Bill


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


On April 11, 2016, 5:03 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 5:03 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46064/#review128315
-----------------------------------------------------------


Ship it!





src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java (line 31)
<https://reviews.apache.org/r/46064/#comment191744>

    It's probably time for some constants to justify these constants (or at least admit that they are lies!).


- Bill Farner


On April 11, 2016, 5:03 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 11, 2016, 5:03 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


Re: Review Request 46064: Removing ResourceVector enum in favor of ResourceType

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46064/#review128301
-----------------------------------------------------------


Ship it!




Master (e95212e) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot retry"

- Aurora ReviewBot


On April 12, 2016, 12:03 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46064/
> -----------------------------------------------------------
> 
> (Updated April 12, 2016, 12:03 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> First step towards consolidating resource definitions: getting rid of custom ResourceVector enum.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java f8c57f9ba7c8d07c14c7a27738e2fba023a2cf03 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java fc9b10b6ecc8bbc3111e516f7a882456f5bc7bd3 
>   src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 1d9d5fc98494892b7d90e97f93db267d42ba7619 
> 
> Diff: https://reviews.apache.org/r/46064/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>