Fix #1735: Add Java Linter to the project #2101

anandwana001 · 2020-11-10T10:06:47Z

Explanation

Fix #1735

For Reviewers

Download .jar file from here - https://github.com/checkstyle/checkstyle/releases/download/checkstyle-8.37/checkstyle-8.37-all.jar
Open terminal
Move to the path where the .jar file is located.
Run the command java -jar checkstyle-8.37-all.jar -c=checkstyle.xml /ABSOLUTE_PATH_TO_OPPIA_ANDROID/app/src/ /ABSOLUTE_PATH_TO_OPPIA_ANDROID/data/src/ /ABSOLUTE_PATH_TO_OPPIA_ANDROID/domain/src/ /ABSOLUTE_PATH_TO_OPPIA_ANDROID/utility/src/ /ABSOLUTE_PATH_TO_OPPIA_ANDROID/testing/src/

Once this PR gets merge, I will add a new guide page in our wiki.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
The PR follows the style guide.
The PR does not contain any unnecessary auto-generated code from Android Studio.
The PR is made from a branch that's not called "develop".
The PR is made from a branch that is up-to-date with "develop".
The PR's branch is based on "develop" and not on any other branch.
The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

.github/workflows/main.yml

checkstyle.xml

.github/workflows/main.yml

app/src/main/java/org/oppia/android/app/databinding/ImageViewBindingAdapters.java

vinitamurthi · 2020-11-11T09:46:40Z

checkstyle.xml

+<!--
+    Checkstyle configuration that checks the Google coding conventions from Google Java Style
+    that can be found at https://google.github.io/styleguide/javaguide.html
+
+    Checkstyle is very configurable. Be sure to read the documentation at
+    http://checkstyle.org (or in your downloaded distribution).
+
+    To completely disable a check, just comment it out or delete it from the file.
+    To suppress certain violations please review suppression filters.
+
+    Authors: Max Vetrenko, Ruslan Diachenko, Roman Ivanov.
+ -->


I am a bit conflicted about this comment. I understand we have picked up the google style xml file but should we be adding their comment or should we add our own and mention that we have picked it up from this particular xml file and reference it? @seanlip can you comment on this - this is more of a open source question?

Could you or @anandwana001 please point me to the repo where this code snippet was taken from, and link to the original file from which these lines were taken? I think I need that info in order to weigh in.

@seanlip From here - https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/google_checks.xml

OK, I did some reading and I think it would be better not to include an LGPL file as part of an Apache repo. Can you write a custom XML checkstyle for Oppia Android rather than using the default?

Also, in general, please never submit any code that's copied verbatim from other places to this repo. All code submitted in PRs must be original (including XML files).

@seanlip This checkstyle file (Google Java Style Guide) is something we need to follow as per Ben's comment - #1735 (comment)

If I create a custom, all I could do is remove comments, else all properties will remain the same as we want the same style to follow.

Just to check, have you confirmed that this correctly fails in CI with missing Java style issues checked in?

@anandwana001 can you confirm this?

yes, it fails correctly.

ah, corrected, I was using sh earlier. fixed it now.

Thanks for checking!

vinitamurthi · 2020-11-11T09:47:18Z

checkstyle.xml

+
+  <property name="severity" value="error"/>
+
+  <property name="fileExtensions" value="java, properties, xml"/>


Does this checker even do XML layout files?

It doesn't work with Android XML layout file.

rt4914

PTAL @anandwana001

app/src/main/java/org/oppia/android/app/databinding/ViewBindingAdapters.java

BenHenning

Thanks @anandwana001! PR LGTM. Looks like the CI failure is a flake. Re-running tests.

Pranav-Bobde · 2021-03-25T15:06:24Z

Can anyone help me resolve this error...Please!

anandwana001 · 2021-03-25T15:42:03Z

@Pranav-Bobde Have you used the correct xml file - https://github.com/checkstyle/checkstyle/blob/14005e371803bd52dff429904b354dc3e72638c0/src/main/resources/google_checks.xml ?

anandwana001 · 2021-03-25T15:44:35Z

Let me know, if you follow the installation steps correctly - https://github.com/oppia/oppia-android/wiki#installation

Pranav-Bobde · 2021-03-25T16:12:56Z

@anandwana001 Yes, I have used the correct file and followed correct steps. In fact, i've deleted, re-forked and done all steps more than 4-5 times now.

anandwana001 · 2021-03-25T16:14:21Z

Could you check the google_checks.xml file is correct and only the required file presented in the system, not the other version of this file in the system?

Pranav-Bobde · 2021-03-25T16:16:29Z

yup just one google_checks.xml here

FareesHussain · 2021-03-25T16:17:30Z

@Pranav-Bobde can you delete the current google_check.xml and add this file
https://github.com/checkstyle/checkstyle/blob/14005e371803bd52dff429904b354dc3e72638c0/src/main/resources/google_checks.xml

it is supposed to be 17KB size here

anandwana001 · 2021-03-25T16:18:22Z

@FareesHussain We had faced this issue earlier, could you confirm is the correct file of 18KB?

FareesHussain · 2021-03-25T16:19:29Z

The correct files is of 17KB

Pranav-Bobde · 2021-03-25T16:21:25Z

@FareesHussain @anandwana001 It's 17.1 Kb only .

anandwana001 · 2021-03-25T16:22:56Z

@Pranav-Bobde Could we confirm it's the same file from that particular we had mentioned in the link?
https://github.com/checkstyle/checkstyle/blob/14005e371803bd52dff429904b354dc3e72638c0/src/main/resources/google_checks.xml

Pranav-Bobde · 2021-03-25T16:32:35Z

@anandwana001

Gone to the link
1
Gone to code and downloaded zip
Extracted & Pasted the file in the Git Directory

FareesHussain · 2021-03-25T16:36:55Z

can you try be deleting and adding the new one

FareesHussain · 2021-03-25T16:38:42Z

In my case, the file is 17.4KB

Pranav-Bobde · 2021-03-25T16:41:28Z

@FareesHussain
Deleted and added. Nothing Changed.

Pranav-Bobde · 2021-03-25T16:42:52Z

@FareesHussain @anandwana001
Would you like to anydesk with me?

anandwana001 · 2021-03-25T16:44:15Z

The issue is, you are downloading code and using the file from maste branch, not from that particular commit we had mentioned in the link.

FareesHussain · 2021-03-25T16:46:24Z

To download open this link https://raw.githubusercontent.com/checkstyle/checkstyle/14005e371803bd52dff429904b354dc3e72638c0/src/main/resources/google_checks.xml

and CTRL + D to download the file

anandwana001 · 2021-03-25T16:47:16Z

Or you can simply right click on raw button and download the file.

Pranav-Bobde · 2021-03-25T16:50:35Z

@anandwana001 @FareesHussain
Ok. It's working . I just copied the content and made a xml file from sublime.
TYSM for your help. Really Appreciate it.

added java linter

be81193

anandwana001 marked this pull request as draft November 10, 2020 10:07

added exec name

aacd296

anandwana001 commented Nov 10, 2020

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

anandwana001 added 4 commits November 10, 2020 16:01

using jar to run command

eff0560

update config file

9c45bfe

checking on java 10

09ec7ad

checking on java 11

6b17885

anandwana001 commented Nov 10, 2020

View reviewed changes

checkstyle.xml Outdated Show resolved Hide resolved

anandwana001 commented Nov 10, 2020

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

anandwana001 added 6 commits November 10, 2020 16:38

adding more indentation to check

ac0a45c

adding property

7671065

updating property

cf8c41b

fixed path for property

96a55f2

update severity to error on custom level

ae6ceca

fix java lint errors

a1f7c0f

anandwana001 commented Nov 11, 2020

View reviewed changes

.github/workflows/main.yml Outdated Show resolved Hide resolved

path reference update

512e234

anandwana001 marked this pull request as ready for review November 11, 2020 02:46

adding all other modules

19bf833

anandwana001 requested review from vinitamurthi and rt4914 November 11, 2020 03:02

anandwana001 assigned rt4914 and vinitamurthi Nov 11, 2020

vinitamurthi reviewed Nov 11, 2020

View reviewed changes

vinitamurthi assigned seanlip Nov 11, 2020

seanlip assigned anandwana001 and unassigned seanlip Nov 11, 2020

rt4914 suggested changes Nov 12, 2020

View reviewed changes

app/src/main/java/org/oppia/android/app/databinding/ViewBindingAdapters.java Outdated Show resolved Hide resolved

rt4914 removed their assignment Nov 12, 2020

anandwana001 added 3 commits February 9, 2021 11:38

fixing script

92c12da

fixing main script

9680162

using #!/bin/bash and error removed

9599fec

anandwana001 assigned BenHenning and unassigned anandwana001 Feb 9, 2021

BenHenning approved these changes Feb 10, 2021

View reviewed changes

BenHenning assigned anandwana001 and unassigned BenHenning Feb 10, 2021

BenHenning enabled auto-merge (squash) February 10, 2021 02:01

BenHenning merged commit f950016 into oppia:develop Feb 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1735: Add Java Linter to the project #2101

Fix #1735: Add Java Linter to the project #2101

anandwana001 commented Nov 10, 2020 •

edited

Loading

vinitamurthi Nov 11, 2020

seanlip Nov 11, 2020

anandwana001 Nov 11, 2020

seanlip Nov 11, 2020

anandwana001 Nov 12, 2020

BenHenning Jan 20, 2021

BenHenning Feb 8, 2021

anandwana001 Feb 9, 2021

anandwana001 Feb 9, 2021

BenHenning Feb 10, 2021

vinitamurthi Nov 11, 2020

anandwana001 Nov 12, 2020

rt4914 left a comment

BenHenning left a comment

Pranav-Bobde commented Mar 25, 2021 •

edited

Loading

anandwana001 commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Mar 25, 2021 •

edited

Loading

FareesHussain commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021


		<property name="severity" value="error"/>

		<property name="fileExtensions" value="java, properties, xml"/>

Fix #1735: Add Java Linter to the project #2101

Fix #1735: Add Java Linter to the project #2101

Conversation

anandwana001 commented Nov 10, 2020 • edited Loading

Explanation

For Reviewers

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rt4914 left a comment

Choose a reason for hiding this comment

BenHenning left a comment

Choose a reason for hiding this comment

Pranav-Bobde commented Mar 25, 2021 • edited Loading

anandwana001 commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

FareesHussain commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Mar 25, 2021 • edited Loading

FareesHussain commented Mar 25, 2021

anandwana001 commented Mar 25, 2021

Pranav-Bobde commented Mar 25, 2021

anandwana001 commented Nov 10, 2020 •

edited

Loading

Pranav-Bobde commented Mar 25, 2021 •

edited

Loading

anandwana001 commented Mar 25, 2021 •

edited

Loading