+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
I’ll take this for now.
> On Jan 30, 2019, at 1:59 PM, Kirk Lund wrote:
>
> I just filed https://issues. apache.org/jira/browse/GEODE-6343 against
> ImportClusterConfigTest.importWouldNotShutDownServer because it failed in a
> precheckin. I'm not sure who to assign this failure to. Any suggestio
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 th
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 20
,
Ken Howe
/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 Se
://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
t; ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> ---
>
> (Updated Sept. 8, 2017, 3:51 p.m.)
>
>
> Revi
: 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
---
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:
>
>
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
/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
---
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
he.org/r/61701/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
with this additional refactoring.
Thanks,
Ken Howe
)
---
Precheckin from earlier ran green.
Re-running precheckin with this additional refactoring.
Thanks,
Ken Howe
+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
>> wrote:
>>
>>> +1
>>>
>>> Sent from my iPhone
>>>
>>
(updated)
---
Re-running precheckin
Thanks,
Ken Howe
ect `--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
t/java/org/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
---
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
---
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
/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
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. T
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61506/#review182518
---
Ship it!
Ship It!
- Ken Howe
On Aug. 9, 2017, 7:02 p.m
---
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
---
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
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61480/
> -------
>
> (Updated Aug. 8, 2017, 9:10 p.m.)
>
>
> Review request for geode, Emily Yeh, Jared Stewart, Ken Howe, Kirk Lund, and
> Patric
(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.
/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 wrote:
>
>
---
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
/apache/geode/test/dunit/rules/GfshShellConnectionRule.java
e7f17ef208a1708f385c7c4041affb70fd309a4c
Diff: https://reviews.apache.org/r/61426/diff/1/
Testing
---
Precheckin ran green
Thanks,
Ken Howe
---
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
---
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
/internal/cli/functions/GetRegionsFunctionTest.java
Lines 17 (patched)
<https://reviews.apache.org/r/61003/#comment256497>
Blank line is not needed between license and package.
- Ken Howe
On July 20, 2017, 5:40 p.m., Jinmei Liao
<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
- K
---
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
---
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
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
/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 St
have enclosing 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-mai
---
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
---
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
---
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
---
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
---
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
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60030/
> -------
>
> (Updated June 12, 2017, 9:14 p.m.)
>
>
> Review request for geode, Emil
---
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
/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., Jar
---
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
-starting precheckin once more
6/8/17: previous precheckin green except for 1 known flaky test
Precheckin started
Thanks,
Ken Howe
---
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
/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
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
the exception of unrelated DUnit tests
Precheckin started
Thanks,
Ken Howe
with the added checks to verify all expected members are
present. However, it wouild be better to eliminate the casue of the "spurious"
error.
- Ken Howe
On June 7, 2017, 9:08 p.m., Jared Stewart wrote:
>
> ---
> This
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
Diff: https://reviews.apache.org/r/59811/diff/1/
Testing
---
Precheckin started
Thanks,
Ken Howe
--
> 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
---
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
views.apache.org/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
&
/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
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., Ja
---
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
---
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
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
iginal), 19 (patched)
<https://reviews.apache.org/r/59544/#comment249352>
This is now an unused import
- Ken Howe
On May 24, 2017, 10:10 p.m., Jared Stewart wrote:
>
> ---
> This is an automatically generated e-
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
---
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
ckin 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
e 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.a
s 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
5/23 - Re-running precheckin after syncing with current state of develop
Thanks,
Ken Howe
--
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
---
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
ding limit.
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
-private 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
------
ExportedLogsSizeInfo, 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
InternalDistrubtedMemberWrapper -> 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
---
9287/diff/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
---
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
---
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
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,
hanges: https://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
far with only DistributedTest still
running
Thanks,
Ken Howe
---
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
---
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
gisteredFunctions) will handle the null
case, and it's more concise.
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/DeployCommandRedeployDUnitTest.java
Lines 129 (patched)
<https://reviews.apache.org/r/59210/#comment248016>
Put this in an @After block
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,
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58996/#review174201
---
Ship it!
Ship It!
- Ken Howe
On May 5, 2017, 11:15 p.m
---
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
/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
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
> 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>
> >
> >
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
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
://reviews.apache.org/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-ma
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:
>
> ---
> This
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-m
eviews.apache.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 automa
ed 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:
> h
1 - 100 of 176 matches
Mail list logo
