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

Add opera_android data #2909

Closed
wants to merge 5 commits into from

Conversation

queengooborg
Copy link
Contributor

Supersedes and closes #1712, also closes #591. This PR is to rebase the original one as well as apply the changes requested by @Elchi3. Conflicting files that reference non-existent Opera Android versions with lower numbers than what has been confirmed to exist have been rounded up to the closest version, however there are MANY files that reference non-existent Opera Android versions (hence why Travis is failing). It's possible that none of such features left are actually supported.

This PR isn't ready to merge until all files have been resolved. Alternatively, all conflicting opera_android version entries can be set to null if desired.

@@ -190,7 +190,7 @@
"version_added": "31"
},
"opera_android": {
"version_added": "31"
"version_added": "32"
Copy link
Contributor

Choose a reason for hiding this comment

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

What is your source for 32? Opera's number is almost always Chrome's number - 13.

Copy link
Contributor Author

@queengooborg queengooborg Oct 4, 2018

Choose a reason for hiding this comment

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

I was setting the versions to the latest version that actually exists (as is confirmed in the other PR, #1712) higher and closest to the original number entered. For example, versions 38-41 don’t exist, so any entries with them get rounded up to 42.

I thought that would be the safest option, but I just found an issue. More details in its own comment.

@@ -142,7 +142,7 @@
"version_added": "39"
},
"opera_android": {
"version_added": "39"
"version_added": "42"
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, where does 42 come from?

I'm going to stop reviewing here because it looks I'll have the same question every time.

@queengooborg
Copy link
Contributor Author

Looks like we have a little bit of an inconsistency on our hands in terms of the version numbers. Opera Android has different version numbers than Opera Desktop, that’s already been established in the discussion on #1712.

https://www.apk4fun.com/history/998/
https://en.wikipedia.org/wiki/Opera_Mobile

However, ChromeStatus.com seems to be referring to Opera Android version numbers as if they were the same as the desktop version numbers, as well as referring to versions way beyond the current released version.

https://www.chromestatus.com/feature/5718547946799104

So...it must be referring to the underlying engine in Opera Android, which follows the desktop version’s numbers?

@Elchi3 Elchi3 added the data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. label Oct 4, 2018
@ExE-Boss
Copy link
Contributor

ExE-Boss commented Oct 4, 2018

I still intend to finish #1712.

@queengooborg
Copy link
Contributor Author

@ExE-Boss I could submit a PR to your branch if you’d like, but I think that we need a little review before continuing on this.

@Elchi3 When you have time, would you be willing to give feedback on the above info regarding the Opera vs. Opera Android versions?

@Elchi3
Copy link
Member

Elchi3 commented Oct 4, 2018

@vinyldarkscratch If @ExE-Boss wants to work on this, I think we should give them the time :) "Stealing" other people's active PRs isn't always helpful. Thanks for your understanding!

I think the version numbers that I mention here #1712 (comment) are correct. Whether or not that's actually doable and makes sense for our data, I don't know yet. Strictly speaking chromestatus seems to do the wrong thing, but maybe they refer to some engine versions. I haven't had the time to dig deeper into this yet.

@queengooborg
Copy link
Contributor Author

Makes sense -- as far as version numbers, a similar case is evident with Microsoft Edge, as described in #2620, where the engine and the application versions are different. As also described, the engine version is preferred, rather than the application version. Personally, I feel that maybe we should keep with that regarding Opera Android (especially since I expect people would believe chromestatus.com's accuracy, and it would also be a little less work :P). But it's all up to you guys in the end, what you think would be best!

@jpmedley
Copy link
Contributor

jpmedley commented Oct 9, 2018

FWIW

@queengooborg queengooborg deleted the browsers/opera-android branch February 2, 2019 01:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:browsers Data about browsers (versions, release dates, etc). This data is used for validation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add browser release data for all browsers
4 participants