Skip to content
This repository has been archived by the owner on Dec 30, 2018. It is now read-only.

Nojquery #41

Closed
wants to merge 4 commits into from
Closed

Nojquery #41

wants to merge 4 commits into from

Conversation

dremonkey
Copy link
Contributor

Removing jQuery dependency

@passy
Copy link
Owner

passy commented Jan 31, 2014

Sorry for the long delay, @dremonkey. Open source stuff is a bit crazy at the moment. :)

Could you have a look at the failing tests?

@dremonkey
Copy link
Contributor Author

No worries. I understand completely. I will take a look as soon as I have some time... hopefully this weekend.

@passy
Copy link
Owner

passy commented Feb 1, 2014

Thanks, @dremonkey!

@jribeiro
Copy link

@dremonkey have you had time to look at the failing tests?

@dremonkey
Copy link
Contributor Author

@jribeiro sorry... got busy. But got around to doing some work on it today. I'm very new to Jasmine so to be honest I'm not sure if what I did is right... but as far as I can tell the first 12 tests are working, one of which is new.

But I am having issues with the last two which I currently have commented out. I will try to get them working this week.

Anyone a little more experienced with writing tests is free to take a look...

@jribeiro
Copy link

No worries. I do have experience in writing tests. I'll take a look in a bit

@JonnyBGod
Copy link
Contributor

+1

2 similar comments
@debuggingfuture
Copy link

+1

@justinhrobbins
Copy link

+1

@passy
Copy link
Owner

passy commented Mar 24, 2014

@jribeiro ping :)

@jspaper
Copy link

jspaper commented Apr 30, 2014

+1

@passy
Copy link
Owner

passy commented Apr 30, 2014

If any of the +1ers wants to pick this up, please feel free to. :)

@kencaron
Copy link

+1, wish I was more comfortable with tests/had more time.

@manikantag
Copy link

+1

Pull request is open around 4 months back. Are there any issues other than tests to merge?
But it says, 'All is well — The Travis CI build passed'.

@passy
Copy link
Owner

passy commented Jun 19, 2014

It should be just about adding tests for the new functionality, the rest
looked solid.

+1

Pull request is open around 4 months back. Are there any issues other than
tests to merge?
But it says, 'All is well — The Travis CI build passed'.


Reply to this email directly or view it on GitHub
#41 (comment).

@pasine
Copy link

pasine commented Jun 27, 2014

+1.
Is there any issue with the current pull request?

@passy
Copy link
Owner

passy commented Jul 2, 2014

The PR has merge conflicts. Happy to review it again after it's rebased.

@passy
Copy link
Owner

passy commented Aug 3, 2014

Closing due to lack of activity. If anyone wants to pick this up, please feel free to open a new PR.

@passy passy closed this Aug 3, 2014
@kenchan0130 kenchan0130 mentioned this pull request Nov 20, 2014
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.

None yet

10 participants