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
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’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
+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
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58751/#review173213
---
Ship it!
Ship It!
- Ken Howe
On April 26, 2017, 8:54 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58742/#review173224
---
Ship it!
Ship It!
- Ken Howe
On April 26, 2017, 5:17 p.m
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58848/#review173384
---
Ship it!
Ship It!
- Ken Howe
On April 28, 2017, 7:40 p.m
c. names don't add clarity to the code. Exceptions would be where the
name might conflict with a keyword, which is not the case here.
- Ken Howe
On April 28, 2017, 6:02 p.m., Jinmei Liao wrote:
>
> ---
> This is an automat
s.apache.org/r/5/#comment246508>
delete line
geode-core/src/main/java/org/apache/geode/cache/DynamicRegionFactory.java
Lines 76 (patched)
<https://reviews.apache.org/r/5/#comment246509>
delete line
- Ken Howe
On May 1, 201
---
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
> 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>
> >
> &
va/org/apache/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 getCo
lt;https://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 au
lt;https://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 au
lt;https://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 au
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
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,
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
/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, 1
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
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
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
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
://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
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
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
> 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>
> >
> >
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
/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
---
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
---
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
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,
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
---
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
---
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
far with only DistributedTest still
running
Thanks,
Ken Howe
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
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,
---
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
---
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
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
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
---
-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
------
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
---
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
--
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
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
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
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
---
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
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
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-
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
---
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
---
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
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
/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
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
&
---
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
--
> 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
Diff: https://reviews.apache.org/r/59811/diff/1/
Testing
---
Precheckin started
Thanks,
Ken Howe
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
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
the exception of unrelated DUnit tests
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
/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
---
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
-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/59852/#review177483
---
Ship it!
Ship It!
- Ken Howe
On June 8, 2017, 9:22 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/60025/#review177737
---
Ship it!
Ship It!
- Ken Howe
On June 12, 2017, 9:50 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/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/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/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/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/60200/#review178313
---
Ship it!
Ship It!
- Ken Howe
On June 19, 2017, 9:45 p.m
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
/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
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
---
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
---
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
<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
/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
---
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
---
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
/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/61409/#review182227
---
Ship it!
Ship It!
- Ken Howe
On Aug. 3, 2017, 5:12 p.m
/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:
>
>
(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.
> 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
---
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/#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/61506/#review182518
---
Ship it!
Ship It!
- Ken Howe
On Aug. 9, 2017, 7:02 p.m
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
/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
---
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
---
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
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
1 - 100 of 176 matches
Mail list logo
