Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add basic Travis CI setup with test script that runs JSHint #961

Merged
merged 5 commits into from Feb 16, 2013

Conversation

Projects
None yet
2 participants
Contributor

brianpeiris commented Feb 15, 2013

I'm hoping this will be accepted as it can be a starting point for adding more automated tests including JSHint and QUnit. Travis CI also has the ability to run PhantomJS so there is a possibility of running more sophisticated UI tests. Of course, the benefit of CI is that we build and test automatically with each commit!

If the pull request is accepted, all you have to do is follow steps 1 and 2 in the Travis CI "Getting Started" document to setup the flot/flot repo. I've already included the .travis.yml configuration file in the pull request. You should then be able to see a (failing!) build at https://travis-ci.org/flot/flot and the status image that I added to the readme should light up.
For example: https://travis-ci.org/brianpeiris/flot

Additionally, you can now run JSHint locally by running the following in your command line:

npm install
npm test

@brianpeiris brianpeiris commented on the diff Feb 15, 2013

.gitignore
@@ -1,2 +1,3 @@
*.min.js
!excanvas.min.js
+node_modules/
@brianpeiris

brianpeiris Feb 15, 2013

Contributor

Ignore the node_modules directory that will be created if you run npm test locally.

@brianpeiris brianpeiris commented on the diff Feb 15, 2013

package.json
@@ -0,0 +1,11 @@
@brianpeiris

brianpeiris Feb 15, 2013

Contributor

Travis CI requires a package.json file.

Contributor

brianpeiris commented Feb 15, 2013

Oh! Bonus! GitHub's integration with Travis is telling you that this pull request is breaking the build :)

Owner

dnschnur commented Feb 15, 2013

This is excellent! The first thing that I have planned once we release 0.8 is to clean up the codebase, bringing it in line with our new (well, a few months old, now) guidelines. This jsHint checker is the perfect way to keep that enforced going forward; I had no idea that Travis integrated so nicely with Github!

So this is a definite merge; the only thing I'm debating is whether to do it now, or immediately after the 0.8 release.

Aside from jsHint I would love to see automated testing for Flot. If you're interested in doing that, it would be a great contribution. If I decided to merge this after 0.8, how much would that obstruct you?

Contributor

brianpeiris commented Feb 15, 2013

Thanks! I would like to start writing tests. Although, why would you have to wait until after 0.8 to merge this? (honest question, I guess I don't understand the situation). Either way, I can continue with the tests on my fork and submit more pull requests as usual. I don't think it would obstruct me, so it's totally up to you.

@ghost ghost assigned dnschnur Feb 15, 2013

Owner

dnschnur commented Feb 16, 2013

I was thinking mainly that we might not want the README badge showing failures all through the 0.8 beta and early release. But that's not a big deal; I'd rather encourage you (and anyone else who wants to contribute tests) to go ahead.

dnschnur added a commit that referenced this pull request Feb 16, 2013

Merge pull request #961 from brianpeiris/master
Add basic Travis CI setup with test script that runs JSHint.

@dnschnur dnschnur merged commit 2cd4f8b into flot:master Feb 16, 2013

Contributor

brianpeiris commented Feb 16, 2013

That's the beauty of TDD: Red, Green, Refactor! Upwards and onward! Thanks for the merge. I'll try to get some JSHint compliance in today, as long as they don't require too many changes (which I don't think they will).

Owner

dnschnur commented Feb 16, 2013

Do you mean actually fixing some of the JSHint issues? Definitely don't do that yet; the number of lines that will touch that overlap with canvas-text changes will make the resulting merge really painful. We also still need to update the style guide with our exact JSHint params; we make some exceptions similar to those used by the jQuery team.

Contributor

brianpeiris commented Feb 16, 2013

Oh, you mean you have a code dump coming? Hmm, I see. I guess I'll wait then.

@brianpeiris brianpeiris referenced this pull request Feb 27, 2013

Closed

JSHint compliance #974

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment