You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2015/03/23 02:37:22 UTC
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
GitHub user robertkowalski opened a pull request:
https://github.com/apache/couchdb-fauxton/pull/326
tooling: automatically test for code style
Currently taking a look if our js coding guidelines are followed
is done manually during the main code review of a PR. This takes
a lot of time for the reviewer and often results in 1-2 more
iterations until the patch is landed, just taking the style issues
into account.
Additionally there are sometimes so much style issues that it
gets hard to focus on the real issues, e.g. the architecture of a
patch or the overall implementation.
This PR enables a precheck that allows to automatically detect if
our styleguide was followed. The errors can now get cleaned up
before the review, saving time of the reviewer and the submitter.
As most style issues won't appear any more in a review the review
focus will change to a discussion that is more related to the
actual implementation, probably resulting in better reviews and
overall code quality.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertkowalski/couchdb-fauxton code-style
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-fauxton/pull/326.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #326
----
commit fd7dfee1b5bd64a3c42645a839c6f7a3b01c5014
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T17:00:37Z
styleguide: fix trailing whitespace
commit 9a8eb544ffe1747e6f0ff58f728e73d853e145ee
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T17:31:27Z
styleguide: 2 spaces intendation
commit 1738eb6cf6e8c79fd0ed2da876743d3ed826e927
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T18:43:27Z
styleguide: disallowEmptyBlocks
commit d1154b99d5fed073a71536df13ef07ae22bcb01e
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T19:11:31Z
styleguide: space around binary operators
commit c05ed342620cf4a86532f26a7b22765f30a40208
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T20:50:44Z
styleguide: requireSpaceAfterBinaryOperators
commit d022b8a696518fcc5215cc9e1b988a7cdb9eedf5
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T20:52:52Z
styleguide: requireSpacesInConditionalExpression
commit d9d810a7f167311092624be0681cc59517dc425b
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T20:57:55Z
styleguide: requireSpaceBeforeBlockStatements
commit ccdbebe14f746e6297454d5ee2a8c4daf37b7278
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T21:09:32Z
styleguide: requireSpaceAfterKeywords
commit 2dffce5c90801ec7b5c65b50fdc3297ffd21de39
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-22T21:20:21Z
tooling: automatically test for code style
Currently taking a look if our js coding guidelines are followed
is done manually during the main code review of a PR. This takes
a lot of time for the reviewer and often results in 1-2 more
iterations until the patch is landed, just taking the style issues
into account.
Additionally there are sometimes so much style issues that it
gets hard to focus on the real issues, e.g. the architecture of a
patch or the overall implementation.
This PR enables a precheck that allows to automatically detect if
our styleguide was followed. The errors can now get cleaned up
before the review, saving time of the reviewer and the submitter.
As most style issues won't appear any more in a review the review
focus will change to a discussion that is more related to the
actual implementation, probably resulting in better reviews and
overall code quality.
commit ac338fbe4dc2b5eeb5bec9238b95208eaff0ac95
Author: Robert Kowalski <ro...@kowalski.gd>
Date: 2015-03-23T01:30:10Z
add style check to gruntfile
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by Madhuka <gi...@git.apache.org>.
Github user Madhuka commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84943835
@robertkowalski Thanks, It will be helpful for all the new comes. Since different styles are used in JS.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84942984
@Madhuka yes, this basically codifies https://github.com/apache/couchdb-fauxton/blob/master/styleguide.md
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:
https://github.com/apache/couchdb-fauxton/pull/326
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84874437
+1 this is great. Could you also create a jshintrc file and we can move all our jshint settings out of the grunt file.
I agree with @kxepal this should be added to travis as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by Madhuka <gi...@git.apache.org>.
Github user Madhuka commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84942722
Hi,
I am new to cocuhDB and I'm looking forward to GSOC. I too know that styling in JS is a pain. I too came across the same issue when we were releasing jaggery[1] (JS server side framework) few years ago. I like to know if there are any preferred style guides/code standards[2,3] for couchdb-fauxton. Knowing style guides/code standards will help us to understand existing code correctly and quickly at the moment. It will help writing correct code styles where automatically tests will pass.
[1] http://jaggeryjs.org/
[2] https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml
[3]http://javascript.crockford.com/code.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84913934
@kxepal @garrensmith it is covered by travis: https://github.com/robertkowalski/couchdb-fauxton/commit/4e1d0d3d5f0386b51c1fb2a79b3e451b901b7c7b#diff-35b4a816e0441e6a375cd925af50752cR515 :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84843441
I think you should add this check to travis as well and let him fail if style is violated. The same I planning to do for documentation today after fixing all the stuff that makes the linter sad. The motivation is to not bypass a stuff that will violate style guide again. You can follow guidelines by yourself, Fauxton team may follow it as well, but new contributors may easily miss some moment not intentionally.
Here is how it works for [Apache Spark](https://github.com/apache/spark/pull/4916#issuecomment-77555917) by the way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84914020
@garrensmith will do in a separate PR - it is on my list!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-fauxton pull request: tooling: automatically test for code...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-fauxton/pull/326#issuecomment-84931773
@robertkowalski cool! I miss that moment (:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---