Since there are more and more contributors joining the project, we are facing the fact that different contributors have their own code style preferences, and some of the codes are changing frequently due to code styles (especially the imports, spaces, etc.), so I'm proposing to do some cleaning works and set up more strict code style rules.
Codes would be much more clean, and code styles are unified, more comfortable to maintain a such big codebase.
Many codes will be modified in the cleaning process.
getters, setters, constructors, and many try-catches;import, such as import java.util.*;java. comes first, org.apache. comes second, etc.,, before {, and between operators, etc.codeStyle.xml that can be imported by other contributorsI hope we could do these things in a single PR, so that:
And I suggest the works could be done right before our next release, after all(or most) pull requests have been merged, and the codebase is frozen, to avoid conflicts;
Anyone who wants to undertake this work, or has different ideas, please leave your comment; if @apache/skywalking-committers have more suggestions, please supplementing the TODOs above by editing this directly, others can create new comment below, thanks;
For the rules you want to set, I am good with them.
And I suggest the works could be done right before our next release, after all(or most) pull requests have been merged, and the codebase is frozen, to avoid conflicts;
I think we may not need to wait for this. It is hard to tell the time, whenever this is ready, please submit the pull request. As we check all these in the CI process, other PRs will be required to change.
+1
Two suggestions
authors, and don't allow to use it, due to the project belongs to the ASF. LOG, logger, LOGGER.Rule, don't use Deprecated API. From https://github.com/apache/skywalking/pull/4318#issuecomment-581966104
If @JohnNiang or any other contributor is interested in this, feel free to leave a comment and open pull requests to fix, thanks :)
@kezhenxu94 Which things left here? As #4337 has been merged.
@kezhenxu94 Which things left here? As #4337 has been merged.
Have updated the TODOs in the first comment of this issue, those left unchecked are unsupported by Checkstyle plugin by default, may need some customization, low priority though
Agree, but is it reasonable to keep this issue open? As there is no tool could do this.
@wu-sheng I'm totally ok to close it for now