Skip to content
This repository was archived by the owner on Jul 24, 2020. It is now read-only.

#1075 Travis Linters #1078

Merged
merged 72 commits into from
Jan 4, 2015
Merged

#1075 Travis Linters #1078

merged 72 commits into from
Jan 4, 2015

Conversation

orenyk
Copy link
Contributor

@orenyk orenyk commented Jan 2, 2015

Resolves #1075, rake task spec still failing most of the time.

orenyk and others added 30 commits December 30, 2014 03:45
test for npm existence

check node version and install linters

run jscs and jshint

fix jscs and jshint globbing issue

change order of script tasks

add imagemagick version check

print ghostscript version

add jshint and jscs config for consistent linting

reduce max errors printed

ignore vendor dir

skip production, staging, and development gems
@mnquintana
Copy link
Contributor

Looks good to me! Is this ready to merge then?

@orenyk
Copy link
Contributor Author

orenyk commented Jan 2, 2015

I'm actually going to remove config/ from the excluded files list and clean those up as well, then this should be good to go.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 2, 2015

Ok, that's done. I want to go over the changes myself one more time but if it all looks good to you I'll merge it in afterwards.

@mnquintana
Copy link
Contributor

Is that the same spec that usually fails for no apparent reason? https://travis-ci.org/YaleSTC/reservations/builds/45691443#L327

@orenyk
Copy link
Contributor Author

orenyk commented Jan 2, 2015

Yup, I'm going to try and debug it once I've finished with this. If I can't figure it out quickly then I'll just comment it out :-\ .

@@ -11,13 +11,32 @@ env:
secure: IHUYMK2spxorl9lUeAbAfT6btuP2qRT615bUnEuUDgYrXf9y1CdQprYWJaygou/+6aWmLL0NnxYSpOxi40bHgMKeUQnTjXVbkkqQ1Tml3cMSsjkBrx7CNUygHvDDzCQCEC6m9uZjUKMZAVzVSWlOQhSMKR7MtdsSvMCrKIgA2pM=

before_install:
# these are all for JS linters, uncomment when they are to be used
# - convert -version
# - gs -v
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you uncomment this and the previous line? Those aren't for the JS linters – they're printing the versions of two of our system dependencies, ImageMagick and GhostScript, to be able to see what Travis is running.

@mnquintana
Copy link
Contributor

Alright everything looks good (just the one change in my code comment)

@squidgetx
Copy link
Contributor

agreed with machiste, i merged master in and dealt with the merge conflicts so this is good to go when those lines are uncommented and you're ready

@mnquintana
Copy link
Contributor

I think we should merge this before any other PRs so Travis can check them for rubocop conformance.

@squidgetx
Copy link
Contributor

yea, i think so too. if those commented lines are all that really needs to be done I can go ahead and do that (and pull in) if necessary

@orenyk
Copy link
Contributor Author

orenyk commented Jan 4, 2015

I'll uncomment those lines and merge this in, thanks guys!

@orenyk
Copy link
Contributor Author

orenyk commented Jan 4, 2015

freaking rake test, merging.

orenyk added a commit that referenced this pull request Jan 4, 2015
@orenyk orenyk merged commit 1502461 into master Jan 4, 2015
@mnquintana mnquintana deleted the 1075_linters_travis branch February 20, 2015 13:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add code quality checking tools / linters to Travis
3 participants