You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iota.apache.org by shiv4nsh <gi...@git.apache.org> on 2016/11/03 19:29:16 UTC
[GitHub] incubator-iota pull request #24: [IOTA-29] Removed the scalastyle Warnings
GitHub user shiv4nsh opened a pull request:
https://github.com/apache/incubator-iota/pull/24
[IOTA-29] Removed the scalastyle Warnings
Removed the Scalastyle warnings and rebased with the new code and it closes #17
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shiv4nsh/incubator-iota IOTA-29
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-iota/pull/24.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 #24
----
----
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes @tonyfaustini @asfgit : Please review this PR asap !
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes : Yes I would do that !
---
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-iota pull request #24: [IOTA-29] Removed the scalastyle Warnings
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-iota/pull/24
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes @asfgit @tonyfaustini I have verified the changes by running the test cases ! Please take a look as soon as possible !
Thanks.
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes : I have made the changes according to your comments please take a look !
Thanks !
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@shiv4nsh please make sure the tests for fey-core are also reflecting the new changes:
```
[error] /Users/barbaragomes/Projects/clean-iota/incubator-iota/fey-core/src/test/scala/org/apache/iota/fey/MonitorSpec.scala:125: type mismatch;
[error] found : IllegalArgumentException
[error] required: Option[Throwable]
[error] monitorRef.underlyingActor.logInfo("path", "STOP", 1, "info", new IllegalArgumentException("Test"))
[error] ^
[warn] one warning found
[error] one error found
[error] (fey-core/test:compileIncremental) Compilation failed
[error] Total time: 15 s, completed Nov 21, 2016 10:38:56 AM
```
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/24
Hi @shiv4nsh,
Thanks for working on it.
quick question: How does adding
{code}
private val DEFAULT_NULL = null
{code}
removes the ScalaStyle warning?
It seems like you just moved the warning to another part of the code
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@shiv4nsh Do you mind opening a JIRA for removing the scalastyle warnings related to the use of ```null``` and **revert** the use of ```private val DEFAULT_NULL = null``` on this PR?
I think this PR should only include what fixed the warnings.
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes : We can re-factor it over the time to make the code more subtle and in a more functional way.
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@shiv4nsh Before opening a PR make sure that at least the basics are working.
1. http://localhost:16666/fey/activeactors is not working.
```
Caused by: java.lang.NullPointerException: null
at org.apache.iota.fey.Trie.org$apache$iota$fey$Trie$$getObject(TrieNode.scala:118)
at org.apache.iota.fey.Trie.print(TrieNode.scala:104)
at org.apache.iota.fey.IdentifyFeyActors$.generateTreeJson(IdentifyFeyActors.scala:87)
at org.apache.iota.fey.FeyUIService$$anonfun$router$1$$anonfun$applyOrElse$1.apply(FeyUIService.scala:46)
at org.apache.iota.fey.FeyUIService$$anonfun$router$1$$anonfun$applyOrElse$1.apply(FeyUIService.scala:43)
at play.api.mvc.ActionBuilder$$anonfun$apply$14.apply(Action.scala:403)
at play.api.mvc.ActionBuilder$$anonfun$apply$14.apply(Action.scala:403)
at play.api.mvc.ActionBuilder$$anonfun$apply$13.apply(Action.scala:371)
at play.api.mvc.ActionBuilder$$anonfun$apply$13.apply(Action.scala:370)
at play.api.mvc.Action$.invokeBlock(Action.scala:498)
at play.api.mvc.Action$.invokeBlock(Action.scala:495)
at play.api.mvc.ActionBuilder$$anon$2.apply(Action.scala:458)
at play.api.mvc.Action$$anonfun$apply$2$$anonfun$apply$5$$anonfun$apply$6.apply(Action.scala:112)
at play.api.mvc.Action$$anonfun$apply$2$$anonfun$apply$5$$anonfun$apply$6.apply(Action.scala:112)
at play.utils.Threads$.withContextClassLoader(Threads.scala:21)
at play.api.mvc.Action$$anonfun$apply$2$$anonfun$apply$5.apply(Action.scala:111)
at play.api.mvc.Action$$anonfun$apply$2$$anonfun$apply$5.apply(Action.scala:110)
at scala.Option.map(Option.scala:146)
at play.api.mvc.Action$$anonfun$apply$2.apply(Action.scala:110)
at play.api.mvc.Action$$anonfun$apply$2.apply(Action.scala:103)
at scala.concurrent.Future$$anonfun$flatMap$1.apply(Future.scala:253)
... 14 common frames omitted
```
2. fey-core/src/main/scala/org/apache/iota/fey/FeyGenericActorReceiver.scala
fey-core/src/main/scala/org/apache/iota/fey/JsonReceiver.scala
private final def downloadJAR - Use Option for var outputStream
Thank you.
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/24
Thank you for fixing the issues with this PR @shiv4nsh.
@tonyfaustini I have tested this PR and everything looks fine now. Please, merge it.
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes : I have minimized them by using this , as the code is tightly coupled hence to remove all the null and using `None` instead would take a while.
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by shiv4nsh <gi...@git.apache.org>.
Github user shiv4nsh commented on the issue:
https://github.com/apache/incubator-iota/pull/24
@barbaragomes : Nice Catch ! Thanks For reviewing ! I have done the changes and now every test case is working fine along with the UI. please review it once again ! !
Test Case Screenshot:
---
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-iota issue #24: [IOTA-29] Removed the scalastyle Warnings
Posted by barbaragomes <gi...@git.apache.org>.
Github user barbaragomes commented on the issue:
https://github.com/apache/incubator-iota/pull/24
Hey @shiv4nsh, before we can merge this PR would you mind implementing the items bellow?
1. File: **fey-core/src/main/scala/org/apache/iota/fey/Ensemble.scala**
```
val performerInfo = performers_metadata.getOrElse(performerID, null)
- if(performerInfo != null){
+ if (Option(performerInfo).isDefined) {
```
Instead of creating an Option after the getOrElse please call **.get(performerID)**, this way the performerInfo is of type Option. (Make sure all the other implications are resolved as well)
2. File: **fey-core/src/main/scala/org/apache/iota/fey/FeyGenericActor.scala**
```
if (Option(scheduler).isDefined)
```
Define global variable scheduler as of type Option and make all of the other necessary changes.
3. File: **fey-core/src/main/scala/org/apache/iota/fey/Monitor.scala**
```
+ if(Option(reason).isDefined){
```
Define parameter reason as of type Option[Throwable] and make the necessary changes
4. File: **fey-core/src/main/scala/org/apache/iota/fey/TrieNode.scala**
```
(Option(root).isDefined)
```
Define variable root as of type Option and make the necessary changes
---
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.
---