You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@twill.apache.org by hsaputra <gi...@git.apache.org> on 2015/06/18 00:10:09 UTC
[GitHub] incubator-twill pull request: (TWILL-136) Override equals and hash...
GitHub user hsaputra opened a pull request:
https://github.com/apache/incubator-twill/pull/42
(TWILL-136) Override equals and hashCode for JvmOptions.DebugOptions to test equality
The current code for JvmOptions.DebugOptions does not have equals and hashCode overriden for equality test.
This would cause fail comparison for DebugOptions.NO_DEBUG when being used in YarnTwillPreparer:
```
final class YarnTwillPreparer implements TwillPreparer {
...
@Override
public TwillPreparer enableDebugging(boolean doSuspend, String... runnables) {
this.debugOptions = new JvmOptions.DebugOptions(true, doSuspend, ImmutableSet.copyOf(runnables));
return this;
}
....
private void saveJvmOptions(Map<String, LocalFile> localFiles) throws IOException {
if ((extraOptions == null || extraOptions.isEmpty()) &&
JvmOptions.DebugOptions.NO_DEBUG.equals(this.debugOptions)) {
// If no vm options, no need to localize the file.
return;
}
...
}
```
...
}
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hsaputra/incubator-twill TWILL-136_override_equals_debugoptions
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-twill/pull/42.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #42
----
commit 737570054596d22f4460f1807ac41e0454305571
Author: hsaputra <hs...@apache.org>
Date: 2015-06-17T22:08:41Z
TWILL-136 Override equals and hashCode for JvmOptions.DebugOptions to test equality
The current code for JvmOptions.DebugOptions does not have equals and hashCode overriden for equality test.
This would cause fail comparison for DebugOptions.NO_DEBUG when being used in YarnTwillPreparer:
final class YarnTwillPreparer implements TwillPreparer {
...
@Override
public TwillPreparer enableDebugging(boolean doSuspend, String... runnables) {
this.debugOptions = new JvmOptions.DebugOptions(true, doSuspend, ImmutableSet.copyOf(runnables));
return this;
}
....
private void saveJvmOptions(Map<String, LocalFile> localFiles) throws IOException {
if ((extraOptions == null || extraOptions.isEmpty()) &&
JvmOptions.DebugOptions.NO_DEBUG.equals(this.debugOptions)) {
// If no vm options, no need to localize the file.
return;
}
...
}
...
}
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-136) Override equals and hash...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/incubator-twill/pull/42#issuecomment-112977856
Cannot reopen due to force update, please check #43
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-136) Override equals and hash...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra closed the pull request at:
https://github.com/apache/incubator-twill/pull/42
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-136) Override equals and hash...
Posted by keith-turner <gi...@git.apache.org>.
Github user keith-turner commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/42#discussion_r32683765
--- Diff: twill-core/src/main/java/org/apache/twill/internal/JvmOptions.java ---
@@ -83,5 +84,28 @@ public boolean doSuspend() {
public boolean doDebug(String runnable) {
return doDebug && (runnables == null || runnables.contains(runnable));
}
+
+ @Override
+ public boolean equals(Object object) {
+ if (this == object) {
+ return true;
+ }
+ if (object == null || !(object instanceof DebugOptions)) {
--- End diff --
I don't think null check is needed.
http://stackoverflow.com/questions/2950319/is-null-check-needed-before-calling-instanceof
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-136) Override equals and hash...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/incubator-twill/pull/42#issuecomment-112968947
Weird test fails, close it to fix the PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-136) Override equals and hash...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/42#discussion_r32687266
--- Diff: twill-core/src/main/java/org/apache/twill/internal/JvmOptions.java ---
@@ -83,5 +84,28 @@ public boolean doSuspend() {
public boolean doDebug(String runnable) {
return doDebug && (runnables == null || runnables.contains(runnable));
}
+
+ @Override
+ public boolean equals(Object object) {
+ if (this == object) {
+ return true;
+ }
+ if (object == null || !(object instanceof DebugOptions)) {
--- End diff --
Thanks for the review, I moved the PR to #43
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---