> On July 13, 2015, 3:04 p.m., Zameer Manji wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/views/Pairs.java, line 
> > 14
> > <https://reviews.apache.org/r/36407/diff/1/?file=1008771#file1008771line14>
> >
> >     Why not move this class to org.apache.aurora.util? Nothing about this 
> > utility class is specific to db views.
> 
> Bill Farner wrote:
>     I prefer to keep code closest to where it's used.  I would have even made 
> the class package private, but it is used in two proximal packages.

SGTM.


- Zameer


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


On July 10, 2015, 3:47 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36407/
> -----------------------------------------------------------
> 
> (Updated July 10, 2015, 3:47 p.m.)
> 
> 
> Review request for Aurora and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> I originally went down this path in an attempt to address AURORA-1395 [1], on 
> a hunch that it was caused by mutating objects coming out of the cache (which 
> are expected to be immutable).  While this did not fix AURORA-1395, it 
> significantly cleaned up DbTaskStore, TaskConfigManager, and DbCronJobStore.
> 
> There are some changes in this patch that are detected as moves, thanks to 
> the large license header and git's move tracking.
> 
> [1] https://issues.apache.org/jira/browse/AURORA-1395
> 
> 
> Diffs
> -----
> 
>   config/findbugs/excludeFilter.xml e1c2503e0c6126c098063e0860f36cebbfe567ee 
>   config/pmd/custom.xml 8ad62280f69db0fe185be0005225cf2d65d62383 
>   src/main/java/org/apache/aurora/GuavaUtils.java 
> 9903299f37179fb4081a67859d4a260527123656 
>   src/main/java/org/apache/aurora/scheduler/storage/db/CronJobMapper.java 
> a5c6e997f7f77c3ed1c760d1ab3023249235bcf5 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbCronJobStore.java 
> 664255fbe6a393af83fcdd8d84eb1987eaff103c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 0f56411d318e6b05d1ea5ae18dd2dc8111264d2c 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 2ec6b2d24b16730841d9d4e30fca49d745546862 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 19f67ad6fd6578f485889398cc45803ca85de58b 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java 
> 7d499af58bdc34167c6b7d4970a2938c996e7ce3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java 
> 07a991d34811256d5ef5504c432fc2b0973ca53f 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java
>  4990af7ce409eb5de20af1a9535bb30abe39ed45 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/AssignedPort.java 
> 0c6442c90289741aa4c59925a6d7a9e4050bf657 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/CronJobWrapper.java
>  5202db5c20dc172586878169440df0543c22daca 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobConfiguration.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbScheduledTask.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/Pairs.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java
>  3e0a2f72389867e61d33cbf4c003f4ec09703b82 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskConfigRow.java 
> 0160ae3049db555cae9e3d3705c1f0daaeef53d4 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskLink.java 
> 52b09a905006bed2a9f01b8beb72772273b5630f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
> 3826d9f275eac6ccadc03df6846a035326482d6e 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  33047281a6fadd13b51ef4041f809c923aa50cf4 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 
> 0b9e3d7d39aa83bbc7e05432638e1b0d50146cda 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
>  00bf3b7df8ae20e0b1c229b1d3c6afcfeb25b403 
> 
> Diff: https://reviews.apache.org/r/36407/diff/
> 
> 
> Testing
> -------
> 
> Note that tests are minimally affected by this change, as it is all hidden 
> behind Store interfaces.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to