-
Notifications
You must be signed in to change notification settings - Fork 481
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
Dev Guide should give more guidance on type safety, code quality, code style, etc. #3950
Comments
@oscardssmith thanks. Here's the chicken scratch from my whiteboard: |
@oscardssmith I updated the title. Are you interested in starting a new branch to help out in this area? Also, please check out the comment I just left for @michbarsinai at #3903 (comment) Thank you to @bsilverstein95 for demonstrating yesterday how one must email the dataverse-dev list when a SQL script is needed to keep one's dev environment in working order after pulling the latest from develop: https://groups.google.com/d/msg/dataverse-dev/l0eDX1vXfvY/ZMRrnQWbBAAJ . From the dev guide we can link to it as an example. |
When we work on this issue let's set expectations for code quality with regard to these issues:
Also, for #3198 let's add a link to https://docs.google.com/a/harvard.edu/presentation/d/1aHjOMy8FIbMVyJEZu_m4ab4ftutgQiC8y6HM47Am2z0/edit?usp=sharing |
Whoops, I didn't mean to close this. 😄 |
Please be aware that just writing some guidelines in a developer guide does not make the ugly or even bad code go away. The (implicit) definition of done for the issues you have been closing is "the bad or ugly code has been removed or corrected". |
@bencomp that's a fair point. Thanks. I'm surprised by your continued interested in the quality of the Dataverse code base and hope we can win you back some day. Thanks for pointing out SonarQube to me. I still haven't tried it myself but I recently heard great things about it at http://illegalargument.com/episode-148-project-glass-tile . For whoever picks this up, let's set expectations first. Write them in a future version of http://guides.dataverse.org/en/4.7/developers/coding-style.html or move the section on type safety to a dedicated page called "Code Quality" (or something) and add them there. Then, once something has been committed about each of the points having to do with code quality, consider looking at the code mentioned in #2674 #2675 #2676 #3551 #3903 in the "develop" branch and (assuming it hasn't been fixed yet) open a fresh issue (remember that smaller pull requests are better so that they are easier to code review) and link to the recommendation about code quality that you just added. |
A suggestion: recently I started using Sonar, which is a code evaluation tool. It gives you suggestions regarding to code quality. It finds possible bugs, dead branches, too complex methods and other code smells. It also provides code coverage statistics. There are multiple ways to use it. I use SonarCloud.io: e.g. https://sonarcloud.io/dashboard?id=pkiraly_metadata-qa-api. I do not know how to integrate it with Travis, but it nicely works together with Github Actions.
Once it is done, the code quality report will be refreshed after every commits, the same way as Travis triggers unit testing and code coverage. |
+1 I've set up SonarQube locally a number of times, but love the idea of SonarCloud and GitHub Actions. |
FWIW: I've set up CodeQL as a github action - no changes to the code itself (unless it was automatic). I'm not sure how these two things compare in terms of what they catch though. |
I don't know what is the difference between CodeQL and Sonar. The point is that these tools give us very good suggestions on how to improve the code. As far as I understand Sonar can not be set without change to pom.xml and the Github Action file. |
We might also take a look at @reviewdog, combining with tools like SpotBugs, PMD, CPD, CheckStyle etc. Many of those tools are included in SonarCube, but might be used on their own. Using sth. like ReviewDog seems interesting for automated pull request reviews, adding comments when rules are violated etc. SpotBugs et al can be used with ReviewDog using https://github.com/mallowlabs/scarfco |
@poikilotherm thanks for adding reviewdog in this PR: Thanks also to @poikilotherm for formatting code consistently with for this issue: I dunno. The dev guide does give some guidance at https://guides.dataverse.org/en/5.11.1/developers/coding-style.html I say we close this issue and open new "small chunk" issues for this or that specific thing we want. |
Related comment: #8771 (comment) |
Related: |
Yes. Closing. |
I understand you wanted to clean up outdated or old issues, Phil, but I wouldn't have said this issue is completed, at least until the "small chunk" issues are created. Could you change to "close as won't fix"? |
Sure, done. 😄 |
We should mention the following
4 spaces
javadoc
dev-list
options -> editor -> code completion -> auto popup
create a settings bundle to import?
We should add sample code, like the code from Netbeans tabs and spaces thing.
We could separate into code style and code quality.
The text was updated successfully, but these errors were encountered: