Skip to content
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

Closed
oscardssmith opened this issue Jun 22, 2017 · 17 comments
Labels
Feature: Developer Guide Type: Suggestion an idea User Role: Hackathon Participant Core developers, frequent contributors, drive-by contributors, etc. Vote to Close: pdurbin
Projects

Comments

@oscardssmith
Copy link
Contributor

oscardssmith commented Jun 22, 2017

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.

@pdurbin
Copy link
Member

pdurbin commented Jun 22, 2017

@oscardssmith thanks. Here's the chicken scratch from my whiteboard:

devguide

@pdurbin pdurbin changed the title More code style fixes should happen Dev Guide should give more guidance on type safety, code quality, code style, etc. Jun 28, 2017
@pdurbin
Copy link
Member

pdurbin commented Jun 28, 2017

@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.

@pdurbin
Copy link
Member

pdurbin commented Jun 29, 2017

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

@pdurbin
Copy link
Member

pdurbin commented Jun 29, 2017

Whoops, I didn't mean to close this. 😄

@pdurbin pdurbin reopened this Jun 29, 2017
@bencomp
Copy link
Contributor

bencomp commented Jun 29, 2017

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".

@pdurbin
Copy link
Member

pdurbin commented Jun 29, 2017

@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.

@pkiraly
Copy link
Member

pkiraly commented Apr 30, 2021

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.
To configure it, here are the steps:

  • register for SonarCloud.io and log in
  • select from the Github projects
  • copy the necessary configurations
  • change pom.xml: add 3 properties (sonar.projectKey, sonar.organisation, sonar.host.url)
  • create a Github Action configuration file (.gitpub/workflows/[some].yml)
  • register the Sonar token in Github's settings/secret
  • check configurations on Github!s action tab

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.

@donsizemore
Copy link
Contributor

+1 I've set up SonarQube locally a number of times, but love the idea of SonarCloud and GitHub Actions.

@qqmyers
Copy link
Member

qqmyers commented Apr 30, 2021

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.

@pkiraly
Copy link
Member

pkiraly commented Apr 30, 2021

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.

@poikilotherm
Copy link
Contributor

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

@pdurbin
Copy link
Member

pdurbin commented Oct 2, 2022

@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.

@pdurbin
Copy link
Member

pdurbin commented Jan 11, 2023

Related comment: #8771 (comment)

@pdurbin
Copy link
Member

pdurbin commented Aug 29, 2023

Related:

@pdurbin
Copy link
Member

pdurbin commented Oct 8, 2023

I say we close this issue and open new "small chunk" issues for this or that specific thing we want.

Yes. Closing.

@pdurbin pdurbin closed this as completed Oct 8, 2023
@bencomp
Copy link
Contributor

bencomp commented Oct 26, 2023

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"?

@pdurbin pdurbin closed this as not planned Won't fix, can't repro, duplicate, stale Oct 26, 2023
@pdurbin
Copy link
Member

pdurbin commented Oct 26, 2023

Sure, done. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Developer Guide Type: Suggestion an idea User Role: Hackathon Participant Core developers, frequent contributors, drive-by contributors, etc. Vote to Close: pdurbin
Projects
Development

No branches or pull requests

7 participants