ForbesLindesay / cabbie

WebDriver for the masses
https://cabbiejs.org/
MIT License
70 stars 11 forks source link

Complete implementation of Web-Driver #5

Closed marcelerz closed 10 years ago

marcelerz commented 10 years ago

I added all unimplemented web-driver calls, added documentation, and tests. Since there were many calls missing (I think about 80%), I needed to make some changes to the interface, resulting in the loss of backwards compatibility to previous versions of cabbie. Please let me know if these big changes are acceptable, and if you are willing to merge them.

marcelerz commented 10 years ago

The tests are failing as the test-runner uses "http://rawgit.com/ForbesLindesay/cabbie/master/test/demoPage.html" as the test-page. However, I have added some elements to the page. Please run the tests locally.

ForbesLindesay commented 10 years ago

Thanks. I'm happy to take your word for this that the tests pass on this one, and fix any issues once it's merged.

The only real issue I have before I merge this, is around licensing. I would like to retain copyright over this repo. I would also like to make sure there is no licensing info in the individual files (it is not required for the license to be valid). Finally, I just wanted to make sure you are aware that this is licensed under MIT, rather than BSD.

I'm more than happy to have you as a contributor in package.json (and anywhere else we list contributors). I just want to be careful to retain ownership of the project.

marcelerz commented 10 years ago

No problem. And, yes. You are right. I somehow recognized it as BSD for some reason. I added the name to the license file to make this clearer if that is ok with you. I know that it is also in the package.json, but this makes it more explicit for a user since I, personally, usually look in the LICENSE file first.

marcelerz commented 10 years ago

I want to point out again that the changes are not backwards compatible. I kept the old functions around for a while, but ended up removing them as they made the code a mess. I bumped up the version to indicate this. Is that ok with you?

ForbesLindesay commented 10 years ago

That's ok. Generally I prefer to bump the version number myself rather than as part of a pull request, but it's not a big issue.

Additionally, this follows SemVer and was bellow 1.0.0 so could release a breaking change on any release. Assuming I'm happy with all of the API once I go through it (I may review it a bit more before I actually release) this can be released as our first stable version though, so 1.0.0 is appropriate.

marcelerz commented 10 years ago

Thank you for merging. I got the coverage to about 82% but want to improve it in the next couple of weeks.