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

Code does not adhere to conventions (as reported by Checkstyle) #32

Open
jody opened this issue Apr 19, 2020 · 8 comments
Open

Code does not adhere to conventions (as reported by Checkstyle) #32

jody opened this issue Apr 19, 2020 · 8 comments
Labels
epic Epic S: Significant Severity: Significant - in the mainline plan

Comments

@jody
Copy link
Contributor

jody commented Apr 19, 2020

Describe the shortcoming
Checkstyle reports errors in java source code files.

To Reproduce
Steps to reproduce the behavior:

  1. Clone repository.
  2. Execute command ./gradlew checkstyle
  3. Open build/reports/checkstyle/main.html
  4. See error reports

Expected behavior
Checkstyle should report no errors with style and coding conventions. Note that ToDo comments are expected to be flagged and do not represent a coding convention violation.

Screenshots
Screen Shot 2020-04-19 at 10 59 06 AM

Development environment:

  • OS: OS X 10.15.4
  • JDK: openjdk 11.0.6 2020-01-14

Additional context
This is an epic and should be the umbrella for creating issues to correct the coding style of individual class files.

@jody jody added documentation Improvements or additions to documentation good first issue Good for newcomers epic Epic labels Apr 19, 2020
acasadoquijada added a commit to acasadoquijada/imagelab that referenced this issue Apr 24, 2020
The code within the imagelab, sound folder and Run.java file
has been refactored to follow the convention as indicated by
the CheckStyle gradle task.

This commit fixes the errors causing CheckStyle not reporting
any error

The most interesting errors were:

* Use of magic numbers.
https://stackoverflow.com/questions/47882/what-is-a-magic-number-and-wh
y-is-it-bad

* Hidden field.
https://stackoverflow.com/questions/7776046/checkstyle-how-to-resolve-h
idden-field-error

* Must match pattern
https://stackoverflow.com/questions/13146771/java-code-conventions-must
-match-pattern-a-za-za-z0-9

* Utility classes should not have a public or default constructor
https://stackoverflow.com/questions/14398747/hide-utility-class-constru
ctor-utility-classes-should-not-have-a-public-or-def

Resolves: MetroCS#32
@robo-jones
Copy link
Contributor

The image depicting the current checkstyle errors is out of date and implies far more errors than exist in the current version. Here is a more up-to-date one that could be utilized instead.
checkstyle report

Additionally, Team 1A would like to take on this epic. Would you prefer that we open new issues for the specific files that still have deficiencies, or should we handle that at our level and submit a single pull request to close this issue (or at least to correct as much as we can)?

@jody
Copy link
Contributor Author

jody commented Oct 21, 2020

Thank you for updating the current state of this issue. If addressing the remaining errors seems too large, you could treat this as an epic and open individual issues that are more conducive to completion.

I am unable to assign an "organization" to an issue, so each individual who wishes to be assigned needs to make a comment directly to this issue.

@cgamble2
Copy link

cgamble2 commented Oct 22, 2020

I would also like to be assigned to this issue. @jody

@DaudAbdi
Copy link

I would like to be assigned this issue

@brianharrity
Copy link

@jody Can I be assigned this as well to work on with my team?

@jody
Copy link
Contributor Author

jody commented Oct 22, 2020

Since these don't associate with a particular team, can you please confirm that the following are all members of the same team?
@robo-jones @cgamble2 @DaudAbdi @brianharrity

@DaudAbdi
Copy link

DaudAbdi commented Oct 22, 2020 via email

@jody
Copy link
Contributor Author

jody commented Oct 22, 2020

Yes, this is correct.
Thanks! Have made the assignments.

@jody jody added this to the Fall 2020 Week 13 Release milestone Oct 23, 2020
@jody jody linked a pull request Nov 13, 2020 that will close this issue
@jody jody removed a link to a pull request Nov 15, 2020
@jody jody added S: Significant Severity: Significant - in the mainline plan and removed good first issue Good for newcomers documentation Improvements or additions to documentation labels Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Epic S: Significant Severity: Significant - in the mainline plan
Projects
None yet
Development

No branches or pull requests

5 participants