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

3950 Dev Guide gives more guidance on type safety, code quality, code style, etc #3971

Conversation

oscardssmith
Copy link
Contributor

Updated coding-style.rst with more info about using netbeans, and type safety tips
Updated documentation.rst to move the better way of installing sphinx first.

This commit improves our developer documentation for writing good code.

Related Issues

  • connects to 3950:Dev Guide Should give more guidance on type safety, code quality, code style, etc

Pull Request Checklist

…s for people not using it

Updated documentation.rst to move the better way of installing sphinx first.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 10.336% when pulling 0822e88 on 3950-Dev-Guide-should-give-more-guidance-on-type-safety,-code-quality,-code-style,-etc into 6f11140 on develop.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oscardssmith thanks for making this pull request but I think I'm going to write this myself after all. Thanks for opening #3950! Let's talk about some of the stuff in my review. Also, rather than 3950-Dev-Guide-should-give-more-guidance-on-type-safety,-code-quality,-code-style,-etc I would have gone for a branch name like 3950-dev-guide. 😄 For branch naming standards, please see http://guides.dataverse.org/en/4.7/developers/version-control.html#create-a-new-branch-off-the-develop-branch .

Like all development teams, the `Dataverse developers at IQSS <https://dataverse.org/about>`_ have their habits and styles when it comes to writing code. Let's attempt to get on the same page. :)

.. contents:: |toctitle|
:local:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want a table of contents on every non-index page. The rule is at http://guides.dataverse.org/en/4.7/developers/documentation.html#table-of-contents and was added as part of #3796.

NetBeans
^^^^^^^^

We highly recommend using Netbeans as a text editor. This makes it significantly easier to commit code that is well formatted and type safe. By default, NetBeans will warn you about some things, but if you want better warnings and code formatting, download this##################TODO ADD FILE######################### zip file, and import it to your nNtBeans settings by going to options, import, and selecting the file. These settings will cause NetBeans to automatically format code that you've worked on whenever you save, point out many common problems in code, and make sure that all indentation uses spaces.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't particularly want this zip file. I don't trust other developers.


As you probably gathered from the :doc:`dev-environment` section, IQSS has standardized on Netbeans. It is much appreciated when you format your code (but only the code you touched!) using the out-of-the-box Netbeans configuration. If you have created an entirely new Java class, you can just click Source -> Format. If you are adjusting code in an existing class, highlight the code you changed and then click Source -> Format. Keeping the "diff" in your pull requests small makes them easier to code review.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bit about "keeping the 'diff' in your pull requests small makes them easier to code review" is very important to me. I don't want a new developer to join the project and say, "Hey, the coding style pages says we use spaces, so I reformatted the entire code base to remove tabs. Here's a pull request." I want a new developer to focus on fixing an actual bug or improving the user experience for end users with a small "diff", a small pull request.

When working on code, you should only reformat the code you're working on. Likewise, you should only improve the type safety of the code you're working on. Eventually, we'll make the code consistent and of higher quality. It won't happen in one summer.


As you probably gathered from the :doc:`dev-environment` section, IQSS has standardized on Netbeans. It is much appreciated when you format your code (but only the code you touched!) using the out-of-the-box Netbeans configuration. If you have created an entirely new Java class, you can just click Source -> Format. If you are adjusting code in an existing class, highlight the code you changed and then click Source -> Format. Keeping the "diff" in your pull requests small makes them easier to code review.
For an example code that follows our styleguide, see this
############TODO INSERT IMAGE################################
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of an image, let's inline a file. For an example of three downloadable JSON files, see http://guides.dataverse.org/en/4.7/installation/oauth2.html#dataverse-side

- Error Fixes
- JDK Migration Support
- Probable Bugs
- Standard Javac Warnings
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't use all of these. Let's have a conversation about which to turn on and why.


Alternative option (Mac/Unix/Windows):

Unless you already have it, install pip (https://pip.pypa.io/en/latest/installing.html)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mac users need to be told to install pip, right?

@pdurbin
Copy link
Member

pdurbin commented Jun 30, 2017

I spoke with @oscardssmith about this just now and he agreed that we'll close this pull request. I'll incorporate all the good ideas in here in future branch with #3950 in the name. Thanks, @oscardssmith !

@pdurbin pdurbin closed this Jun 30, 2017
@poikilotherm poikilotherm deleted the 3950-Dev-Guide-should-give-more-guidance-on-type-safety,-code-quality,-code-style,-etc branch July 1, 2020 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants