Ducktape codestyle #8098
Ducktape codestyle #8098
Conversation
…cktape-codestyle # Conflicts: # modules/ducktests/tests/ignitetest/services/ignite_app.py # modules/ducktests/tests/ignitetest/services/ignite_spark_app.py # modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py # modules/ducktests/tests/ignitetest/services/utils/ignite_aware_app.py # modules/ducktests/tests/ignitetest/services/utils/ignite_config.py # modules/ducktests/tests/ignitetest/tests/add_node_rebalance_test.py # modules/ducktests/tests/ignitetest/tests/discovery_test.py # modules/ducktests/tests/ignitetest/tests/pme_free_switch_test.py # modules/ducktests/tests/ignitetest/tests/spark_integration_test.py # modules/ducktests/tests/ignitetest/tests/utils/version.py
|
|
||
| def _version(self, node_or_version): | ||
| @staticmethod | ||
| def __version__(node_or_version): |
There was a problem hiding this comment.
Methods with double underscore on both sides have special meaning. Such methods are mixins and doesn't suppose to be just private methods. Private methods have another naming: __version, without trailing underscores.
There was a problem hiding this comment.
Yep, you are right
| return os.path.join(self.home(version, project=project), "bin", script_name) | ||
|
|
||
| def _version(self, node_or_version): | ||
| @staticmethod |
There was a problem hiding this comment.
there is no need for staticmethod decorator as the method is never used beyond this class
There was a problem hiding this comment.
Both pycharm and pylint generate warnings on it.
| def master_log_path(self, node): | ||
| return "org.apache.spark.deploy.worker.Worker" | ||
|
|
||
| @staticmethod |
There was a problem hiding this comment.
Do we really need static methods in our code? I don't see any case in our code where usage of them is required. It just adds more sugar without need. Also some code styles consider them evil, e.g. Google Guide.
There was a problem hiding this comment.
Despite the fact that in mentioned above codestyle staticmethods are not welcome in code, linters generate warnings about unused self. And usage of @staticmethod is a preferrable way to mitigate this case. This is so common in my practice and widely accepted in a python's community.
3 participants
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.