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

Fix chromium version handling #43

Merged
merged 1 commit into from
Mar 27, 2019
Merged

Fix chromium version handling #43

merged 1 commit into from
Mar 27, 2019

Conversation

joe81
Copy link
Contributor

@joe81 joe81 commented Mar 26, 2019

Fix the handling of chrome version with additional version informations like "Using PPAPI flash.\n73.0.3683.75".

@kapoorlakshya
Copy link
Collaborator

Hi @joe81, thanks for your PR! I would've never caught that since Flash is disabled on my browsers now 🙂.

Your change looks good, and it makes me think that maybe we should use something more along the lines of \d+\.\d+\.\d+\.\d+ (https://rubular.com/r/QSFR4enA1Wgq2N). The Regex in your PR will work as long as the version is at the end of the string. However, if Google decides to add some other info after it, such as "x64" or "win", it'll cease to work. Just thinking ahead to keep it low maintenance.

Let me know what you think.

@joe81
Copy link
Contributor Author

joe81 commented Mar 27, 2019

Hello @kapoorlakshya , thank you for your quick answer. Your regex is much more robust than the previous one. From my point of view you should use it. I would close my PR because it is obsolete. How do you see that? When could you make the change available?

@kapoorlakshya
Copy link
Collaborator

@joe81 Feel free to update your PR with the preferred regex, and then squash the commits. I'll approve and merge it it once the specs pass on Travis. Hoping to do a v3.7.2 release very soon..

Fix the handling of chrome version with additional version informations like:

Using PPAPI flash.\n73.0.3683.75
Google Chrome 73.0.3683.75_win
@joe81
Copy link
Contributor Author

joe81 commented Mar 27, 2019

@kapoorlakshya I've adjusted the PR.

@kapoorlakshya
Copy link
Collaborator

Thank you!

@kapoorlakshya kapoorlakshya merged commit 77e9e5e into titusfortner:master Mar 27, 2019
@joe81 joe81 deleted the fix-chromium-product-version-handling branch March 28, 2019 05:53
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.

None yet

2 participants