publiclab / plots2

a collaborative knowledge-exchange platform in Rails; we welcome first-time contributors! :balloon:
https://publiclab.org
GNU General Public License v3.0
961 stars 1.83k forks source link

Setup linting in the project #1729

Closed ryzokuken closed 6 years ago

ryzokuken commented 7 years ago

A lot of the code in the plots2 codebase doesn't exactly conform to a specific set of design guidelines and therefore might be difficult to read and comprehend. Also, because a chunk of the code is unlinted, it opens it up to things like trailing whitespaces which more often than not leads to noise in new PRs when smarter text editors try to auto remove them. I personally believe a good linting setup would add up to our already huge set of tests, further improving our sanity checks to make sure that the submitted code doesn't only "not break the code", the newly written code is formatted cleanly and according to our guidelines and thus in a consistent manner.

P.S. We will also be able to easily churn out a ton of easy linting FTOs.

/cc @jywarren what do you say about this?

jywarren commented 7 years ago

I love it! The one thing I'd like to request is that we tweak the actual text that the linting offers to new contributors, to be extra friendly. Sometimes such systems will be like "FORMATTING WRONG, FIX IT TO GET ACCEPTED" (exaggeration of course) and we want something more like:

"Hi! One thing we try to do is format our code consistently and cleanly, so it's very readable, especially for newcomers! Here are some ways you could tweak the formatting of your contribution to help keep things tidy. Thanks!"

What do you think?

On Thu, Oct 26, 2017 at 9:31 AM, Ujjwal Sharma notifications@github.com wrote:

A lot of the code in the plots2 codebase doesn't exactly conform to a specific set of design guidelines and therefore might be difficult to read and comprehend. Also, because a chunk of the code is unlinted, it opens it up to things like trailing whitespaces which more often than not leads to noise in new PRs when smarter text editors try to auto remove them. I personally believe a good linting setup would add up to our already huge set of tests, further improving our sanity checks to make sure that the submitted code doesn't only "not break the code", the newly written code is formatted cleanly and according to our guidelines and thus in a consistent manner.

P.S. We will also be able to easily churn out a ton of easy linting FTOs.

/cc @jywarren https://github.com/jywarren what do you say about this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/plots2/issues/1729, or mute the thread https://github.com/notifications/unsubscribe-auth/AABfJ_k6jKlvonXCeseV9L91S90AhJIOks5swImUgaJpZM4QHktN .

ViditChitkara commented 7 years ago

@ryzokuken , @jywarren are we going to use any linting gem like robocup or something?? Also could you please give me a bit more info about formatting the code, so that I could start working on it

ryzokuken commented 7 years ago

@viditchitkara take a look at https://github.com/publiclab/plots2/pull/1733. Perhaps we will merge this and then make separate issues for linting each file. Basically, we're planning on solving these issues one file at a time. Let us know if you had anything else in mind.

ViditChitkara commented 7 years ago

Oops I guess I didn't see the PR😅😅