search

dogecoin / libdohj

Java library for adding altcoin support to bitcoinj
Apache License 2.0
107 stars 89 forks source link

proposal: checkstyle (& maven central) #50

Open bfu4 opened 3 years ago

bfu4 commented 3 years ago

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.

rnicoll commented 3 years ago

Sorry, missed that this was a PR not an issue, had scheduled this for when I had time to write code. Will review ASAP.

bfu4 commented 3 years ago

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.