You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by krichter722 <gi...@git.apache.org> on 2015/03/09 02:27:36 UTC
[GitHub] storm pull request: added missing @Override annotations
GitHub user krichter722 opened a pull request:
https://github.com/apache/storm/pull/460
added missing @Override annotations
help to detect inheritance issues when generics are used more intensively
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/krichter722/storm override
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/460.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 #460
----
commit 5c0c40ff368508e1e735cfca58114b1fc63bcf42
Author: Karl-Philipp Richter <kr...@aol.de>
Date: 2015-03-09T01:26:30Z
added missing @Override annotations (help to detect inheritance issues when generics are used more intensively)
----
---
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] storm issue #460: [STORM-3000] added missing @Override annotations
Posted by krichter722 <gi...@git.apache.org>.
Github user krichter722 commented on the issue:
https://github.com/apache/storm/pull/460
@d2r I rebased the branch, removed changes in classes generated by the Thrift compiler and included all subprojects. Since checkstyle is now in place I reviewed all `maxAllowedViolations` in POMs in order to avoid future missing `@Override`s.
I can reduce the `maxAllowedViolations` values is a separate PR, but I think it makes sense to include them here.
Please reopen this PR (I can't do it) can trigger Travis.
---
[GitHub] storm pull request: added missing @Override annotations
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/460#issuecomment-78388527
@krichter722 thanks for the pull request, if you could remove the changes you made to the generated thrift code the rest seems fine.
---
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] storm pull request: added missing @Override annotations
Posted by Parth-Brahmbhatt <gi...@git.apache.org>.
Github user Parth-Brahmbhatt commented on the pull request:
https://github.com/apache/storm/pull/460#issuecomment-77788298
I don't think we should modify the thrift generated files, any way we could tell thrift to generate these annotation? If not, can we leave those files as is given they should never be modified manually anyways so @override will not provide any value in those cases.
---
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] storm pull request: added missing @Override annotations
Posted by nathanmarz <gi...@git.apache.org>.
Github user nathanmarz commented on the pull request:
https://github.com/apache/storm/pull/460#issuecomment-77798677
-1
Agree with Parth. Modifying the generated files is out of the question. The Thrift compiler should be patched to add those annotations if it doesn't have an option for that already.
---
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] storm pull request: added missing @Override annotations
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/460
---
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] storm pull request: added missing @Override annotations
Posted by knusbaum <gi...@git.apache.org>.
Github user knusbaum commented on the pull request:
https://github.com/apache/storm/pull/460#issuecomment-100333108
@krichter722 Any news on this?
---
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] storm pull request: added missing @Override annotations
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/storm/pull/460#issuecomment-167630407
@krichter722 this pull request has been inactive for months, so I will be closing it soon. Please reopen the pull request if you would like to address the comments. If you do, please create a Jira for this work and resolve conflicts with the master branch.
---
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.
---