Skip to content
This repository has been archived by the owner on Oct 10, 2021. It is now read-only.

Fix bug parsing chrome version string "30..latest" #67

Merged
merged 1 commit into from Aug 7, 2014
Merged

Fix bug parsing chrome version string "30..latest" #67

merged 1 commit into from Aug 7, 2014

Conversation

feross
Copy link
Collaborator

@feross feross commented Aug 6, 2014

This fixes a bug where parsing the following browser field in .zuul.yml
config fails:

browsers:
  - name: chrome
    version: 35..latest

Casting ‘beta’ to a number results in the end version being set to NaN.

@feross
Copy link
Collaborator Author

feross commented Aug 6, 2014

Not sure why the travis tests failed. They passed locally for me.

@defunctzombie
Copy link
Owner

The tests will fail cause your branch doesn't have access to the saucelabs keys. I am not sure how to fix this so PRs stop failing. Maybe a Travis config.

@feross
Copy link
Collaborator Author

feross commented Aug 6, 2014

Oh well. What do you think about the actual change? It's simple enough :)

@defunctzombie
Copy link
Owner

Can we do something smarter? Like ignore anything that is not a number maybe? This seems like a very specific fix so if another word pops up we will need to patch that too.

This fixes a bug where parsing the following browser field in .zuul.yml
config fails:

browsers:
- name: chrome
version: 35..latest

Casting ‘beta’ to a number results in the end version being set to NaN.
@feross
Copy link
Collaborator Author

feross commented Aug 7, 2014

@defunctzombie I was just matching what is already done in another place in this same file. See https://github.com/defunctzombie/zuul/blob/master/lib/flatten_browser.js#L72

But, I agree that it would be nice to do something smarter. I can update this PR to fix both uses.

@feross
Copy link
Collaborator Author

feross commented Aug 7, 2014

PR updated.

defunctzombie added a commit that referenced this pull request Aug 7, 2014
Fix bug parsing chrome version string "30..latest"
@defunctzombie defunctzombie merged commit 78d71dd into defunctzombie:master Aug 7, 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

2 participants