-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: checkstyle (& maven central) #50
base: main
Are you sure you want to change the base?
Conversation
Sorry, missed that this was a PR not an issue, had scheduled this for when I had time to write code. Will review ASAP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor tweaks (I've raised suggestions, so should be easy to do), but generally I think looks good.
core/src/main/java/org/libdohj/params/AbstractDogecoinParams.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/libdohj/params/AbstractDogecoinParams.java
Outdated
Show resolved
Hide resolved
apply suggestion for documentation comment for AbstractDogeParams#getChainID Co-authored-by: Ross Nicoll <[email protected]>
Awesome! I'm going to commit these documentation suggestions now, and update some of the code to be compliant with the checkstyle. After those, there are about 88 missing comments (in the core module), where most should be relatively easy to add. |
update documentation comment per suggestion for AbstractDogecoinParams#getSerializer Co-authored-by: Ross Nicoll <[email protected]>
What's new
Reasoning
I was browsing through Github and came upon this repository. This is quite cool. This repository is also already ery well documented (yay!). I'm suggesting a checkstyle to enforce maintenance on that, as well as just to have the feature to check the style of future additions. As the checkstyle is very strict, the code does not compile due to a few missing documentation features. This can be checked out by pulling the branch and running any of the checkstyle tasks for the projects that include Java files (all projects except
wallettemplate
).The checkstyle follows a template used by Sun, and also used by hyperledger's sdk, except is modified to allow the ternaries and magic numbers.
I'd love to be able to contribute to this, but I'd not go forth with doing the documentation without pitching it here. I'm creating this pull request to see if this is something that would like to be done and if so, track progress and discussion here. With that, this PR is marked as a draft 😃.
The checkstyle excludes the file generated by the protobufs.
Other Notes
Eventually, I think #28 has a good point. This requires credentials but could be done as well. Of course, I cannot implement that, as it should be published under dogecoin's group.
I know the pain of checkstyles, so I'm sure seeing a pull request adding a checkstyle is probably nothing short to painful, especially for a repository with so many files and is already very well documented. However, I think with something like libraries, the possibility of contributions can be greeted with enforcing a style followed throughout the project, as well as is just a nice feature to have.
Since this repository is not actively maintained (it doesn't seem to be, at least) this PR can be viewed as more of a casual suggestion, and if it came off as more harsh (it being a checkstyle, and all), it did not mean to be.