> On May 8, 2020, at 3:41 PM, Jinmei Liao wrote:
>
> It's not the test code, it's the "start locator" command itself.
>
Same rules apply I think. If the method is deprecated don’t use it. If the
method using it is intentionally using the deprecated behavior it too should be
deprecated,
It's not the test code, it's the "start locator" command itself.
On Fri, May 8, 2020 at 2:27 PM Jacob Barrett wrote:
>
> > On May 8, 2020, at 1:55 PM, Jinmei Liao wrote:
> >
> > What's the recommendation for the legitimate usage of the deprecated
> > property? In my case, a gemFire property is
> On May 8, 2020, at 2:30 PM, Kirk Lund wrote:
>
> I'll file a ticket to improve this class.
There is a project geode-unsafe, perhaps it belongs in there, if not already
there. Even geode-common works. That way it can be used in both test and
production code. It is useful for addressing
I'll file a ticket to improve this class.
On Fri, May 8, 2020 at 2:24 PM Jacob Barrett wrote:
>
>
> > On May 8, 2020, at 1:42 PM, Kirk Lund wrote:
> >
> > I've been using this class in geode-core mostly for tests:
> >
> > package org.apache.geode.internal.cache.util;
> >
> > import
> On May 8, 2020, at 1:55 PM, Jinmei Liao wrote:
>
> What's the recommendation for the legitimate usage of the deprecated
> property? In my case, a gemFire property is deprecated, but "start locator"
> command still has an option to turn on that property (the option is
> deprecated as well,
> On May 8, 2020, at 1:42 PM, Kirk Lund wrote:
>
> I've been using this class in geode-core mostly for tests:
>
> package org.apache.geode.internal.cache.util;
>
> import org.apache.geode.cache.execute.Execution;
>
> @SuppressWarnings({"unchecked", "unused"})
> public class UncheckedUtils
What's the recommendation for the legitimate usage of the deprecated
property? In my case, a gemFire property is deprecated, but "start locator"
command still has an option to turn on that property (the option is
deprecated as well, but we are still obligated to keep it in the code). In
this case,
I've been using this class in geode-core mostly for tests:
package org.apache.geode.internal.cache.util;
import org.apache.geode.cache.execute.Execution;
@SuppressWarnings({"unchecked", "unused"})
public class UncheckedUtils {
public static T cast(Object object) {
return (T) object;
}
Regarding...
*> public interface A is returning Region and not Region is that an
acceptable suppression?*
I think Geode API/Framework (e.g. PDX) (production) code should be very
explicit and *not* use *rawtypes*, or even *wildcard* types, for that
matter, as far as possible.
To be clear, I am
It is slightly different, but here is a case I recently found.
List versionTags = versions.getVersionTags();
This method is internal though.
public List getVersionTags() {
return Collections.unmodifiableList(this.versionTags);
}
Thanks,
Mark
> On May 8, 2020, at 1:25 PM, Mark Hanson wrote:
How does everyone feel about situations involving raw types?
Such as public interface A is returning Region and not Region is that an
acceptable suppression?
Thanks,
Mark
> On May 8, 2020, at 1:20 PM, John Blum wrote:
>
> Agreed, but the following (inside tests) does not work in all cases,
Agreed, but the following (inside tests) does not work in all cases, i.e.
Region region...
Particularly if "region" is passed to a method with a different type
signature.
I am trying to find/think of the situation I encounter from time to time,
even when I use the *wildcard* signature (i.e.
> On May 8, 2020, at 1:08 PM, Kirk Lund wrote:
>
> Actually there is an alternative to using @SuppressWarnings for Mockito
> mocks:
>
> Region region = cast(mock(Region.class));
>
> private static T cast(Object object) {
> return (T) object;
> }
The cast method will need to suppress
Actually there is an alternative to using @SuppressWarnings for Mockito
mocks:
Region region = cast(mock(Region.class));
private static T cast(Object object) {
return (T) object;
}
Personally, I'd prefer to see warnings or workarounds like above than see
lots of @SuppressWarnings littered
I should clarify...
The use of ["rawtypes", "unchecked"] are quite common in test code and
"unused" is common in API/Framework (production) code
While *tests* make more liberal use of these checks ("unused" is
questionable (!), though), I think all of these checks should be used
judiciously in
@Donal -
Well, if you have code like...
public void someMethod(@Nullable Object value) {
Assert.notNull(value, "...");
value.invokeSomeMethod();
...
}
The compiler will often *warn* you that value might be null without a
proper null check. That is, not all IDEs recognize "valid"
> On May 8, 2020, at 12:41 PM, Donal Evans wrote:
>
> Is there a consensus on what constitutes a benign warning? I think the only
> times I use @SuppressWarnings is in parameterized tests to suppress the
> unused method warning for the parameters method, or for unchecked casts, as
> below:
>
>
> You can disable inspection for a warning that is otherwise benign...
>
Is there a consensus on what constitutes a benign warning? I think the only
times I use @SuppressWarnings is in parameterized tests to suppress the
unused method warning for the parameters method, or for unchecked casts,
Let's try this again :P.
+1 to Kirk's comments. Plus...
Another tip (for IJ IDEA users, probably same for Eclipse and other IDEs):
You can disable inspection for a warning that is otherwise benign (or not
correct) *rather than* unnecessarily annotating the code with
@SuppressWarnings.
Another tip (for IJ IDEA users, probably same for Eclipse and other IDEs):
You can disable an inspection wher
On Fri, May 8, 2020 at 11:52 AM Michael Oleske wrote:
> For context, here is an example of PR that added warnings as error
> https://github.com/apache/geode/pull/4816. Here is the
I submitted and PR a while ago that started making warnings errors on certain
modules. Through lazy consensus it was approved and merged. I would love to
apply it to more. To set a baseline, I tried to fix most of the deprecated and
other warnings as possible in the effected code. Many were so
For context, here is an example of PR that added warnings as error
https://github.com/apache/geode/pull/4816. Here is the JIRA
https://issues.apache.org/jira/projects/GEODE/issues/GEODE-7869
-michael
On Fri, May 8, 2020 at 11:45 AM Kirk Lund wrote:
> I'm reviewing lots of PRs which are
>
I'm reviewing lots of PRs which are
adding @SuppressWarnings("deprecation"), etc. I think this is a bad trend.
We should not be suppressing warnings in our code. That's hiding a problem
that we'll need to or want to deal with in the future. Also, if you add
several of these suppress annotations
23 matches
Mail list logo
