Skip to content
This repository was archived by the owner on Jan 31, 2018. It is now read-only.

Infer version 915695#142

Closed
willkg wants to merge 3 commits intomozilla:masterfrom
willkg:infer-version-915695
Closed

Infer version 915695#142
willkg wants to merge 3 commits intomozilla:masterfrom
willkg:infer-version-915695

Conversation

@willkg
Copy link
Member

@willkg willkg commented Sep 17, 2013

This changes the firefox os detection code to also infer the firefox os platform and browser version from the browser parts of the user agent.

There's some data in production that has "wrong gecko versions", so the migration ignores those figuring I don't know what it is if it doesn't have a correct Gecko version.

To test, run the tests, run the migration forwards and backwards and test the web form with FirefoxOS browser.

r?

This adds some code to infer the FxOS version from the Gecko
version specified in the browser parts of the user agent. This
isn't a perfect way to get the version--there's a bunch of data
that doesn't have the right Gecko versions. However, it's
probably accurate for certain Gecko versions.

This includes a data migration that backfills the data.
This makes it easier to know which migration had what output. At some
point p, we'll nix the output. But it's super helpful for knowing what
happened during deployments.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just overwrites the same thing above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah... I'm not sure what the first one is there for. I'll nix it.

@willkg
Copy link
Member Author

willkg commented Sep 18, 2013

Sorry this was sloppy. I didn't do a quality pass on it because I needed to run off and make dinner. I should have waited to do the pr until later.

@willkg
Copy link
Member Author

willkg commented Sep 18, 2013

^^^ That fixes the duplicate code and the None thing.

@rlr
Copy link
Contributor

rlr commented Sep 18, 2013

ok, that fixed it. i ran migration forwards and backwards and things worked with my (limited) data. also tests passed. r+

@willkg
Copy link
Member Author

willkg commented Sep 18, 2013

Landed: ff988bd

@willkg willkg closed this Sep 18, 2013
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.

2 participants