Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permit changing default user agent #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

allspiritseve
Copy link

No description provided.

@gshutler
Copy link
Owner

gshutler commented Sep 5, 2016

What is the use case for this change?

@allspiritseve
Copy link
Author

It can be useful in some situations to know when a user agent can't be parsed correctly. My preference is to return an empty string instead of Mozilla/4.0 (compatible). As not everyone has this preference, it makes sense to allow a configurable default rather than having to monkeypatch UserAgent::DEFAULT_USER_AGENT.

@gshutler
Copy link
Owner

gshutler commented Sep 5, 2016

I can see something along those lines being useful. For example, if returning nil for a user agent that couldn't be detected you could do something like:

if ua = UserAgent.parse(user_agent)
  # Something based on known UA
else
  # Fallback for unknown UA
end

I could also see a parse! variant that raised an error if the UA was unknown being useful for similar patterns.

WDYT? Yours is a non-breaking change but I'm already considering a major version for #40 so I could include a breaking change along these lines with that into a major version.

@allspiritseve
Copy link
Author

@gshutler sounds good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants