Re: Bug Numbers and Trac Numbers in comments

+1 - What Jake said. > On Feb 19, 2019, at 5:21 PM, Jacob Barrett wrote: > > Comments that don’t provide meaningful context beyond what is already > expressed in the code should be removed. A number to a system that the > general public can’t access is not meaningful. Delete or replace with

Re: Remove usages of deprecated classes

I agree that we have too many uses of code deprecated in our own code base. I also agree with the idea that we should not introduce new usages of deprecated classes. So if someone is modifying a class that uses deprecated classes, should the deprecations be refactored out or is it OK to leave

Re: Javadoc errors in org/apache/geode/cache/configuration/RegionAttributesType.java

I merged that for you this earlier this morning > On Dec 20, 2018, at 4:10 PM, Aditya Anchuri wrote: > > Okay. Please go ahead and merge, I can’t since I’m not a committer. > > On Thu, Dec 20, 2018 at 3:00 PM Kirk Lund wrote: > >> Looks good. Thanks! I added my approval. >> >> On Thu, Dec

Review Request 62248: GEODE-3539: Add test for missing coverage of status locator command

, Ken Howe

Re: Review Request 62189: GEODE-2817: consolidate authorize(*) methods

/Function.java Lines 120 (patched) <https://reviews.apache.org/r/62189/#comment261259> You are expecting an internal object to be passed in a public API here. I think this can be deleted from the current change set as in my search it doesn't appear to be used. - Ken Howe On Sept. 8

Re: Review Request 62132: GEODE-3277: Fix error path constructors of Launcher inner State classess

://reviews.apache.org/r/62132/diff/2-3/ Testing (updated) --- Precheckin was green except for known problem with a new protobuf test and 1 apparently flaky test in HARQueueNewImplDUnitTest that passed on a second run. Thanks, Ken Howe

Re: Review Request 62179: Test Rule Fix: clean up client DS when using LocatorServerStartupRule

-- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62179/ > --- > > (Updated Sept. 8, 2017, 3:51 p.m.) > > > Review request for geode

Re: Review Request 62132: GEODE-3277: Fix error path constructors of Launcher inner State classess

: https://reviews.apache.org/r/62132/diff/2/ Changes: https://reviews.apache.org/r/62132/diff/1-2/ Testing --- Precheckin is green Thanks, Ken Howe

Re: Review Request 62132: GEODE-3277: Fix error path constructors of Launcher inner State classess

gging that isn't normally necessary. - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62132/#review184852 ------- On Sept. 6, 2017, 8:10 p.m., Ken Howe wrote: > > -

Review Request 62132: GEODE-3277: Fix error path constructors of Launcher inner State classess

geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java a9ce889006800523505dace6e0b4696c9911d205 Diff: https://reviews.apache.org/r/62132/diff/1/ Testing --- Precheckin is green Thanks, Ken Howe

Re: Review Request 61972: GEODE-3445: Convert connect acceptance test to DUnit test

/internal/cli/commands/ConnectCommandWithSSLTest.java Line 267 (original), 264 (patched) <https://reviews.apache.org/r/61972/#comment260207> Spelling: "conect" corrected elsewhere, but missed this one. - Ken Howe On Aug. 29, 2017, 4:53 p.m., Jar

Re: Review Request 61974: GEODE-2859: Fix race condition in ShowDeadlockDUnitTest

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61974/#review184151 --- Ship it! Ship It! - Ken Howe On Aug. 29, 2017, 10:46 p.m

Re: Review Request 61701: GEODE-3277: Fix error path constructors of Launcher inner State classess

diff/4-5/ Testing (updated) --- Precheckin from earlier ran green. 8/23/17: Re-running precheckin with this additional refactoring. 8/24/17: Once more around the precheckin merry-go-round Thanks, Ken Howe

Re: Review Request 61701: GEODE-3277: Fix error path constructors of Launcher inner State classess

with this additional refactoring. Thanks, Ken Howe

Re: Review Request 61701: GEODE-3277: Fix error path constructors of Launcher inner State classess

) --- Precheckin from earlier ran green. Re-running precheckin with this additional refactoring. Thanks, Ken Howe

Re: Gitbox enables the full GitHub workflow

+1 Yes, let’s make the move > On Aug 22, 2017, at 11:21 AM, Nabarun Nag wrote: > > +1 > > On Tue, Aug 22, 2017 at 11:15 AM Kirk Lund wrote: > >> +1 to move all our repos to gitbox >> >> On Tue, Aug 22, 2017 at 11:08 AM, Jacob Barrett

Re: Review Request 61701: GEODE-3277: Fix error path constructors of Launcher inner State classess

(updated) --- Re-running precheckin Thanks, Ken Howe

Re: Review Request 61701: GEODE-3277: Fix error path constructors of Launcher inner State classess

mand correctly shows the locator is online when given the correct `--port=...` value. - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61701/#review183145 --- On Aug. 16, 2017, 9:21 p.m., Ken Howe wrot

Review Request 61701: GEODE-3277: Fix error path constructors of Launcher inner State classess

/apache/geode/test/dunit/rules/GfshShellConnectionRule.java e7f17ef208a1708f385c7c4041affb70fd309a4c Diff: https://reviews.apache.org/r/61701/diff/1/ Testing --- Precheckin is in progress. Thanks, Ken Howe

Re: Review Request 61627: GEODE-3437: Fix list and describe region tests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61627/#review183042 --- Ship it! Ship It! - Ken Howe On Aug. 16, 2017, 5:24 p.m

Re: Review Request 61627: GEODE-3437: Fix list and describe region tests

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61627/#review183037 --- Ship it! Ship It! - Ken Howe On Aug. 14, 2017, 10:40 p.m

Re: Review Request 61671: GEODE-3328: fix testAddGemFirePropertyFileToCommandLineNew on Windows

/internal/cli/commands/GfshCommandJUnitTest.java Line 411 (original), 411 (patched) <https://reviews.apache.org/r/61671/#comment258982> Why was this test renamed? Not really a problem but on the surface looks unneeded. - Ken Howe On Aug. 15, 2017, 7:29 p.m., Kirk Lund

Re: Review Request 61599: GEODE-3328: fix a test failure on windows.

independant constructs is always good. I didn't try this out myself on a Windows machine but the fix looks good. - Ken Howe On Aug. 11, 2017, 10:42 p.m., Jinmei Liao wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61480/#review182443 --- Ship it! Ship It! - Ken Howe On Aug. 8, 2017, 9:10 p.m

Re: Review Request 61480: GEODE-3330: user needs CLUSTER:MANAGE:QUERY permission to create a CQ.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61480/#review182440 --- Ship it! - Ken Howe On Aug. 8, 2017, 9:10 p.m., Jinmei Liao

Re: Review Request 61487: GEODE-3407: fix deadlock between JMX and reconnect

(cache, cacheServer, ...) - Ken Howe On Aug. 8, 2017, 12:19 a.m., Kirk Lund wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 61417: GEODE-3328: adding ssl-truststore-type to the config

/ConfigurationProperties.java Lines 691 (patched) <https://reviews.apache.org/r/61417/#comment258115> Is this declaration needed? It doesn't appear to be used anywhere - Ken Howe On Aug. 3, 2017, 9:15 p.m., Jinmei Liao

Re: Review Request 61409: GEODE-3328: simplify GfshParserRule

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61409/#review182227 --- Ship it! Ship It! - Ken Howe On Aug. 3, 2017, 5:12 p.m

Review Request 61426: GEODE-3277: Fix error path constructors of inner State classes of the Launchers

unit/rules/GfshShellConnectionRule.java e7f17ef208a1708f385c7c4041affb70fd309a4c Diff: https://reviews.apache.org/r/61426/diff/1/ Testing --- Precheckin ran green Thanks, Ken Howe

Re: Review Request 61196: GEODE-3326: Fix intermittent ConcurrentDeployDUnitTest failure

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61196/#review181674 --- Ship it! Ship It! - Ken Howe On July 27, 2017, 10:22 p.m

Re: Review Request 60985: GEODE-3217: Reimplement gfsh query as a single-step command

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60985/#review181069 --- Ship it! Ship It! - Ken Howe On July 20, 2017, 5:54 p.m

Re: Review Request 60985: GEODE-3217: Reimplement gfsh query as a single-step command

<https://reviews.apache.org/r/60985/#comment256480> Seems there's opportunity for more tests in here, for instance, queryWithInvalidRegionName, queryInvalidExceptionThrown, etc. Have you checked coverage, in particular for the new classes QueryCommand, and QueryInterceptor - Ken Ho

Re: Review Request 60977: GEODE-3251: make JMX test rules more robust

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60977/#review180959 --- Ship it! Ship It! - Ken Howe On July 19, 2017, 5:14 p.m

Re: Review Request 60666: geode-3166: remove the uncalled getCredential method

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60666/#review180683 --- Ship it! Ship It! - Ken Howe On July 17, 2017, 3:49 p.m

Re: Review Request 60666: geode-3166: remove the uncalled getCredential method

should be marked as @Deprecated for the upcoming release rather than immediately removing it. Removing the @Deprecated annotation on the 3-arg method is appropriate as this is now the preferred method. - Ken Howe On July 5, 2017, 7:47 p.m., Jinmei Liao wrote

Re: Review Request 60348: GEODE-3103: GfshRule no longer clutters output

/test/dunit/rules/gfsh/GfshRule.java Line 103 (original), 102 (patched) <https://reviews.apache.org/r/60348/#comment253035> Yeah, That's even better than my suggestion! - Ken Howe On June 23, 2017, 6:01 p.m., Jared Stewart

Re: Review Request 60348: GEODE-3103: GfshRule no longer clutters output

ing quotes on the value arg. -OR- change those tests to be consistent with adding the quotes here. - Ken Howe On June 21, 2017, 10:51 p.m., Jared Stewart wrote: > > --- > This is an automatically generated e-mail. To reply, visit:

Re: Review Request 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60200/#review178313 --- Ship it! Ship It! - Ken Howe On June 19, 2017, 9:45 p.m

Re: Review Request 60200: GEODE-3095: fix parameter type mismatch between the diskstore command and controller

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60200/#review178305 --- Ship it! Ship It! - Ken Howe On June 19, 2017, 9:12 p.m

Re: Review Request 60202: GEODE-3056: fix the message for invalid partition-resolver

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60202/#review178302 --- Ship it! Ship It! - Ken Howe On June 19, 2017, 7:08 p.m

Re: Review Request 60199: GEODE-3092: fix specifiedDefaultValue for cacheLoader and cacheWriter

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60199/#review178272 --- Ship it! Ship It! - Ken Howe On June 19, 2017, 4:09 p.m

Re: Review Request 60010: GEODE-3062: replace SecurityService after loading cluster config

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60010/#review177928 --- Ship it! Ship It! - Ken Howe On June 13, 2017, 4:29 p.m

Re: Review Request 60025: GEODE-3060: Introduce JUnit rule for testing the fully-assembled GFSH

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60025/#review177737 --- Ship it! Ship It! - Ken Howe On June 12, 2017, 9:50 p.m

Re: Review Request 59961: GEODE-3048: Introduce a rule to identify tests that require GEODE_HOME

/dunit/rules/RequiresGeodeHome.java Lines 28 (patched) <https://reviews.apache.org/r/59961/#comment251315> Many of us are in the habit of putting '\n' in message strings, but I think using LINE_SEPARATOR would be better. - Ken Howe On June 9, 2017, 11:35 p.m., Jared Stewart

Re: Review Request 59852: GEODE-2632: make SecurityService immutable to improve client/server performance

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59852/#review177483 --- Ship it! Ship It! - Ken Howe On June 8, 2017, 9:22 p.m

Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

-starting precheckin once more 6/8/17: previous precheckin green except for 1 known flaky test Precheckin started Thanks, Ken Howe

Re: Review Request 59893: GEODE-3032: Fix CI failure of CommandOverHttpDUnitTest

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59893/#review177303 --- Ship it! Ship It! - Ken Howe On June 7, 2017, 11:13 p.m

Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

/diff/2-3/ Testing (updated) --- 6/7/17: re-started precheckin precheckin is green with the exception of unrelated DUnit tests * re-starting precheckin once more Precheckin started Thanks, Ken Howe

Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

Good suggestion, I do think it helps readability. - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59811/#review177230 --- On June 7, 2017

Re: Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

ons/SizeExportLogsFunctionTest.java cc5e7d5256741ad0a48ff87c7f989a18b90f7f03 Diff: https://reviews.apache.org/r/59811/diff/2/ Changes: https://reviews.apache.org/r/59811/diff/1-2/ Testing (updated) --- 6/7/17: re-started precheckin Precheckin started Thanks, Ken Howe

Review Request 59811: GEODE-2420: add file-size-limit param to the ExportLogsController

Diff: https://reviews.apache.org/r/59811/diff/1/ Testing --- Precheckin started Thanks, Ken Howe

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

-- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59692/ > ------- > > (Updated June 1, 2017, 5:21 p.m.) > > > Review request for geode, Emi

Re: Review Request 59643: GEODE-3006: reduce the frequency of ping request and reduce the loglevel of login/logout messages

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59643/#review176546 --- Ship it! Ship It! - Ken Howe On May 31, 2017, 10:46 p.m

Re: Review Request 59686: GEODE-2983: correctly handling --J option value that has ", " inside.

g/r/59686/ > ------- > > (Updated May 31, 2017, 5:42 p.m.) > > > Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and > Patrick Rhomberg. > > > Repository: geode > > >

Re: Review Request 59692: GEODE-2925: add target for resource operation for finer grained security

/ResourcePermission.java Line 77 (original), 95 (patched) <https://reviews.apache.org/r/59692/#comment249906> I think it would be better to use Region.SEPARATOR instead of "/". - Ken Howe On May 31, 2017, 8:55 p.m., J

Re: Review Request 59611: GEODE-2989: Improve mechanism for scanning the classpath to find gfsh commands

g/r/59611/#comment249761> This new method in a product class is only used in a test class. It would be better to move this out of product code to the test where it's needed. - Ken Howe On May 26, 2017, 10:02 p.m., Jared S

Re: Review Request 59642: GEODE-3000: do not have jetty log at debug level

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59642/#review176378 --- Ship it! Ship It! - Ken Howe On May 30, 2017, 6:07 p.m

Re: Review Request 59653: GEODE-2420 Warn a user if they try to export too much data. Added gfsh ref docs for --file-size-limit option.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59653/#review176362 --- Ship it! Ship It! - Ken Howe On May 30, 2017, 9:21 p.m

Re: Review Request 59542: GEODE-2974: rename ResultBuilder methods: GemFire -> Geode

a4ff6d210ed7f3ba167057de9a0a5023 Diff: https://reviews.apache.org/r/59542/diff/2/ Changes: https://reviews.apache.org/r/59542/diff/1-2/ Testing (updated) --- 5/30 - precheckin restarted precheckin is running Thanks, Ken Howe

Review Request 59542: GEODE-2974: rename ResultBuilder methods: GemFire -> Geode

geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/LuceneIndexCommands.java 5e17f6e55e6726f22cc39ddaba1ec608ca5cab9d Diff: https://reviews.apache.org/r/59542/diff/1/ Testing --- precheckin is running Thanks, Ken Howe

Re: Review Request 59505: GEODE-2964: add common-collections to gfsh dependencies

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59505/#review175871 --- Ship it! Ship It! - Ken Howe On May 23, 2017, 10:11 p.m

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running 5/23 - Re-running precheckin after syncing with current state of develop Thanks, Ken Howe

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

the renames correctly, and the diff from my repo ended up with some extra changes. - Ken Howe On May 23, 2017, 2:52 p.m., Ken Howe wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

ess - all green so far with only DistributedTest still running 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running 5/23 - Re-running precheckin after syncing with current state of develop Thanks, Ken Howe

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59287/#review175719 --- On May 22, 2017, 6:14 p.m., Ken Howe wrote: > > --- > This is an automatically generated e-mail. To reply, vis

Re: Review Request 59457: GEODE-2959: remove DistributedSystem instance in tearDown

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59457/#review175674 --- Ship it! Ship It! - Ken Howe On May 22, 2017, 6:22 p.m

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

. See note from jstewart's review - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59287/#review175127 --- On May 22, 2017, 6:14

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

ate so it can spied it in the tests. - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59287/#review175023 -------

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

ogsSizeInfo, and now just retiurn a single Long as the result (or error if the size check fails). - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59287/#review175123 ------

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

rnalDistrubtedMemberWrapper -> InternalDistributedMemberWrapper See reply above regarding changes to InternalDistributedMember - Ken --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59287/#review175282

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

2-3/ Testing (updated) --- Precheckin is in progress - all green so far with only DistributedTest still running 5/22 - Precheckin being re-run. Currently Green expcet for the known LocatorLauncher test failures. DistributedTests still running Thanks, Ken Howe

Re: Review Request 59384: GEODE-1930: temporarily disable verifySystemNotifications

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59384/#review175566 --- Ship it! Ship It! - Ken Howe On May 19, 2017, 5:06 p.m

Re: Review Request 59299: GEODE-2874: Fix StringIndexOutOfBoundsException while initializing logger

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59299/#review175041 --- Ship it! Ship It! - Ken Howe On May 15, 2017, 10:31 p.m

Re: Review Request 59299: GEODE-2874: Fix StringIndexOutOfBoundsException while initializing logger

che.org/r/59299/#comment248354> I'd favor a more descriptive test name that won't prompt digging through a JIRA to figure out what the intent is. "testGeode2874_nameWithoutExtensionDoesntThrow"? - Ken Howe On May 15, 2017, 10:04 p.m

Re: Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

tps://reviews.apache.org/r/59287/diff/1-2/ Testing --- Precheckin is in progress - all green so far with only DistributedTest still running Thanks, Ken Howe

Review Request 59287: GEODE-2420: Enable export logs size estimation and user warning

nly DistributedTest still running Thanks, Ken Howe

Re: Review Request 59246: GEODE-2876: reset isGfshVM flag to false when tearing down tests using CliCommandTestBase.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59246/#review175004 --- Ship it! Ship It! - Ken Howe On May 12, 2017, 10:12 p.m

Re: Review Request 59210: GEODE-2912: Hot deploy for functions in deployed Jars

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59210/#review174826 --- Ship it! Ship It! - Ken Howe On May 12, 2017, 6:06 p.m

Re: Review Request 59098: GEODE-2876: add logging to diagnose parser test failure in Jenkins

added org.apache.logging.log4j.Logger as an import rather than specifying the full class path in the declaration. - Ken Howe On May 9, 2017, 3:32 p.m., Jinmei Liao wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 59035: GEODE-2883: Fix GFSH gc heap size output

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59035/#review174171 --- Ship it! Ship It! - Ken Howe On May 5, 2017, 11:19 p.m

Re: Review Request 59035: GEODE-2883: Fix GFSH gc heap size output

/cli/util/BytesToStringTest.java Line 27 (original), 19 (patched) <https://reviews.apache.org/r/59035/#comment247206> Unused import? - Ken Howe On May 5, 2017, 11:08 p.m., Jared Stewart wrote: > > --- > This is an automati

Re: Review Request 59035: GEODE-2883: Fix GFSH gc heap size output

atched) <https://reviews.apache.org/r/59035/#comment247204> Test doesn't belong in this class - Ken Howe On May 5, 2017, 11:08 p.m., Jared Stewart wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 58682: GEODE-2662: Missing keys cause columns to shift in gfsh table display.

> On May 5, 2017, 10:03 p.m., Ken Howe wrote: > > geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DataCommands.java > > Line 91 (original) > > <https://reviews.apache.org/r/58682/diff/5-7/?file=1702040#file1702040line94> > > > >

Re: Review Request 58682: GEODE-2662: Missing keys cause columns to shift in gfsh table display.

trace" Then remove all the local declarations of: LogWriter logger = cache.getLogger(); - Ken Howe On May 5, 2017, 9:22 p.m., Patrick Rhomberg wrote: > > --- > This is an automatically generated e-mail

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

to to product code. No new comments or issues on this batch of diffs. - Ken Howe On May 3, 2017, 10:10 p.m., Kirk Lund wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

g/r/5/#comment247071> Was this intended to be 'ForceReattemptException ignore'? - Ken Howe On May 3, 2017, 10:10 p.m., Kirk Lund wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

n.java Line 2001 (original), 1960-1961 (patched) <https://reviews.apache.org/r/5/#comment247042> Swap these lines to keep the comments contiguous - Ken Howe On May 3, 2017, 10:10 p.m., Kirk Lund wrote: > > --- >

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

edit: This can probably be ignored if you've reverted the DistTX* changes - Ken Howe On May 3, 2017, 10:10 p.m., Kirk Lund wrote: > > --- > This is an automatically generated e-mail. To re

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

che.org/r/5/#comment246892> Suggest rewording this comment "For a long time conflict checks were turned off ..." - Ken Howe On May 3, 2017, 10:10 p.m., Kirk Lund wrote: > > --- > This is an automatically ge

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

d and cancelled are considered correct, and we use both of them. - Ken Howe On May 3, 2017, 10:10 p.m., Kirk Lund wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

/InternalLocator.java Lines 97-98 (original), 96-97 (patched) <https://reviews.apache.org/r/5/#comment246860> In the javadoc, missing a closing '}' and a ';' between the first two statments - Ken Howe On May 2, 2017, 12:06 a.m., Kirk Lund

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

views.apache.org/r/5/#comment246856> typo - correspnds to my note on line 111 - Ken Howe On May 2, 2017, 12:06 a.m., Kirk Lund wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > h

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

empty inner class we won't get a stack trace at all. So just throw the new exception and let the default constructor fill in the stack trace and casue. If the intent was to not have the cause filled in then couldn't we just throw a new zero-arg CacheException? - Ken Howe On May 2, 2017, 12:

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

ode/cache/query/internal/index/AbstractIndex.java Line 1223 (original), 1256 (patched) <https://reviews.apache.org/r/5/#comment246788> typo: remove '//' that are left over from the original comment - Ken Howe On May

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

tps://reviews.apache.org/r/5/#comment246693> more comment formatting - Ken Howe On May 2, 2017, 12:06 a.m., Kirk Lund wrote: > > --- > This is an automat

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

tps://reviews.apache.org/r/5/#comment246693> more comment formatting - Ken Howe On May 2, 2017, 12:06 a.m., Kirk Lund wrote: > > --- > This is an automat

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

tps://reviews.apache.org/r/5/#comment246693> more comment formatting - Ken Howe On May 2, 2017, 12:06 a.m., Kirk Lund wrote: > > --- > This is an automat

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

che/geode/cache/query/internal/QueryUtils.java Line 1503 (original), 1409 (patched) <https://reviews.apache.org/r/5/#comment246589> For consistency, suggest renaming this method to getConditionedRelationshipIndexResultsExpandedToTopOrCGJLevel (similar to the renamed getConditionedI

Re: Review Request 58888: GEODE-2632: change dependencies on GemFireCacheImpl to InternalCache

> On May 1, 2017, 10:28 p.m., Ken Howe wrote: > > geode-core/src/main/java/org/apache/geode/cache/AttributesFactory.java > > Line 237 (original), 232 (patched) > > <https://reviews.apache.org/r/5/diff/1/?file=1704078#file1704078line237> > > >

Re: Review Request 58682: GEODE-2662: Missing keys cause columns to shift in gfsh table display.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58682/#review173528 --- Ship it! Ship It! - Ken Howe On April 27, 2017, 6:30 p.m

  1   2   >