> On Dec. 31, 2014, 7:36 p.m., kturner wrote: > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReader.java, > > line 78 > > <https://reviews.apache.org/r/29502/diff/2/?file=804700#file804700line78> > > > > was putting @Override on same line as method decleration intentional? > > Christopher Tubbs wrote: > Probably best to just format and organize imports for all the changed > files. I noticed a lot of other formatting issues, too. > > Corey Nolet wrote: > Not sure why intelli-j defaults to this behavior but it's fixed. > > Christopher Tubbs wrote: > Import order is something that our formatting standards don't even > address, I just noticed the change and thought it unusual. > > Corey Nolet wrote: > This is something we worked out on Fluo early on and I believe the static > changing from the top of the imports to the bottom was a result of that- > though I'm surprised, unless Keith has multiple profiles for his import > orders, why we wouldn't have noticed this sooner in his patches. > > See https://github.com/fluo-io/fluo/wiki/Contributing#coding-guidelines > > kturner wrote: > > unless Keith has multiple profiles for his import orders > > I use two eclipse workspaces w/ different config, one for Fluo and one > for Accumulo. I try my best to avoid making changes unrelated to the task I > am working. > > Corey Nolet wrote: > Christopher, when I read over your comment early this morning, I read it > as "Import order is something that our formatting standards don't even > address, [maybe it's time we address them.]" I referenced what we did on > Fluo just to present the idea of putting those standards on the site, not to > say any they need to be changed. > > > I use two eclipse workspaces w/ different config, one for Fluo and one > for Accumulo > > It makes sense now. I was wondering why the deltas were only happening > for me. I just assumed Mike and yourself were using the defaults that were > already being used in Accumulo.
Corey, yes I guess I was hinting at an implied 'maybe it's time we address them'. But, I was also wanted to express that you shouldn't feel it necessary to make the change to comply with my preferences, since we haven't actually established a particular standard. In my initial comment, I didn't realize you weren't using Eclipse, but in my second comment, I was pointing out that even if you were using Eclipse with our formatter, it still wouldn't have (necessarily) addressed the import order if you had modified them from the defaults. - Christopher ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29502/#review66493 ----------------------------------------------------------- On Jan. 6, 2015, 10:54 a.m., Corey Nolet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29502/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2015, 10:54 a.m.) > > > Review request for accumulo, Christopher Tubbs, Eric Newton, Josh Elser, and > kturner. > > > Bugs: ACCUMULO-3458 > https://issues.apache.org/jira/browse/ACCUMULO-3458 > > > Repository: accumulo > > > Description > ------- > > ACCUMULO-3458 Propagating scan-time authorizations through the > IteratorEnvironment so that scan-time iterators can use them. > > > Diffs > ----- > > > core/src/main/java/org/apache/accumulo/core/client/ClientSideIteratorScanner.java > 4903656 > core/src/main/java/org/apache/accumulo/core/client/ScannerBase.java 335b63a > core/src/main/java/org/apache/accumulo/core/client/impl/OfflineScanner.java > 2552682 > core/src/main/java/org/apache/accumulo/core/client/impl/ScannerImpl.java > 666a8af > core/src/main/java/org/apache/accumulo/core/client/impl/ScannerOptions.java > 9726266 > > core/src/main/java/org/apache/accumulo/core/client/impl/TabletServerBatchReader.java > 2a79f05 > > core/src/main/java/org/apache/accumulo/core/client/mock/MockScannerBase.java > 72cb863 > > core/src/main/java/org/apache/accumulo/core/iterators/IteratorEnvironment.java > 9e20cb1 > > core/src/test/java/org/apache/accumulo/core/client/impl/ScannerImplTest.java > be4d467 > > core/src/test/java/org/apache/accumulo/core/client/impl/TabletServerBatchReaderTest.java > PRE-CREATION > > core/src/test/java/org/apache/accumulo/core/iterators/DefaultIteratorEnvironment.java > 94da7b5 > > core/src/test/java/org/apache/accumulo/core/iterators/FirstEntryInRowIteratorTest.java > fa46360 > > core/src/test/java/org/apache/accumulo/core/iterators/user/RowDeletingIteratorTest.java > 4521e55 > > core/src/test/java/org/apache/accumulo/core/iterators/user/TransformingIteratorTest.java > 4cebab7 > > server/base/src/test/java/org/apache/accumulo/server/iterators/MetadataBulkLoadFilterTest.java > 4a45e99 > > server/base/src/test/java/org/apache/accumulo/server/replication/StatusCombinerTest.java > a9801b0 > > server/monitor/src/main/java/org/apache/accumulo/monitor/servlets/trace/NullScanner.java > bf35557 > > server/tserver/src/main/java/org/apache/accumulo/tserver/TabletIteratorEnvironment.java > d1fece5 > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Compactor.java > 869cc33 > > server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/ScanDataSource.java > fe4b16b > test/src/main/java/org/apache/accumulo/test/functional/AuthsIterator.java > PRE-CREATION > test/src/test/java/org/apache/accumulo/test/ScanIteratorIT.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29502/diff/ > > > Testing > ------- > > Wrote an integration test to verify that ScanDataSource is actually setting > the authorizations on the IteratorEnvironment > > > Thanks, > > Corey Nolet > >
