> On Nov. 5, 2014, 12:43 a.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java, line > > 318 > > <https://reviews.apache.org/r/27598/diff/1/?file=749747#file749747line318> > > > > why not initialize this inline? > > Maxim Khutornenko wrote: > Not sure I get it. Don't we need a handle to pull these mappings into > test? > > Kevin Sweeney wrote: > I'm proposing making the field `@VisibleForTesting` directly and reading > it in the test. Slightly less code but either approach works. > > Maxim Khutornenko wrote: > Ah, I see what you meant now. Inlining isn't going to work here as > closures use writeBehind- instance fields and I'd rather not move that much > code into the constructor. > > Kevin Sweeney wrote: > writeBehind is an instance field so they can use it from there. I'm > proposing (simplified) > > ```java > class OurClass { > private final Storage storage; > @VisibleForTesting > final Map<Field, Closure<Entry>> HANDLERS = ImmutableMap.builder() > .add( > REMOVE_TASK, > (entry) -> storage.write( > (storeProvider) -> storeProvider.getTaskStore().removeTask( > entry.getRemoveTask().getTaskId()))) > ... > .build(); > } > ```
Right, it still does not pass checkstyle though: "Variable 'logEntryReplayActions' must be private and have accessor methods." - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27598/#review59893 ----------------------------------------------------------- On Nov. 5, 2014, 1:52 a.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27598/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2014, 1:52 a.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-912 > https://issues.apache.org/jira/browse/AURORA-912 > > > Repository: aurora > > > Description > ------- > > Fixing the log replay for the job update history pruner. > > Also, refactored LogStorage replay routine to fully test all > LogEntry/LogEntry.TRANSACTION handlers. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > cbab75964052a950e1b868b3a53eb15fadb31cb7 > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java > 8eb5c3f7c542206066b39a09911c7df01a43bee7 > > Diff: https://reviews.apache.org/r/27598/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
