search

kevinelliott / agent_orange

Parse and process User Agents like a secret one
126 stars 36 forks source link

Some changes #10

Closed brain-geek closed 12 years ago

brain-geek commented 12 years ago

This commits enable travis-ci support for different rubies, not only 1.8.7, And add rake tasks for checking real-world useragents. So far, it passes completely(except Chrome-Safari), so it may be rewritten into real spec.

kevinelliott commented 12 years ago

Thanks for the contribution. The first commit looks fine. As for the user agent testing, we already have a basic rspec spec to test for users useragents, but it does not check the whole list. I'd like to include your changes, but it needs to be reworked to:

1) Not depend on an external website dependency (this could easily break due to network issues, etc and would fail the test; just not necessary). You could possibly pull down that content and store it in the gem, and simply add an attribution to the site that provided it.

2) We already have an allagents.xml that contains an incredible number of users useragents in it. Perhaps you could rework your code to use that?

3) Merge your code into the user useragent testing in /spec. We have a separate one for bot useragent testing.

Once you've done this, please do resubmit the pull request and I'll review and merge it in.

brain-geek commented 12 years ago

Done all suggestions.

I have thought about using allagents.xml , but it is realy outdated - for example, it has only one real chrome user agent, and zytrax.com list is updated often and has a lot of different versions to test with.

kevinelliott commented 12 years ago

Your updated commit looks fantastic. Thank you for your contribution. I'll merge this request now.

kevinelliott commented 12 years ago

brain-geek:

It looks like the test fails on Ruby 1.9.3. Now I know that 1.9.3 is development, but it might be good to hack in support for it. It appears to be some UTF-8 issue. I'm not personally familiar with 1.9.3 yet (max I've used is 1.9.2). Perhaps you can take a look and submit a new pull request to fix.

Here's the build log from travis-ci - http://travis-ci.org/#!/kevinelliott/agent_orange/builds/383453/L27