Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the data here isn't quite wrong, but it's definitely not complete. It looks like, in the transition from Presto to Blink, Opera did indeed remove support for unprefixed
@keyframes
at version 15. Chrome unprefixed@keyframes
in version 43, which suggests that Opera unprefixed in version 30 (i.e., Chrome version number - 13). This is corroborated by the Chrome Platform Status entry.Instead, I think we need to restructure the Opera data with support statements in this order:
"version_added": "30"
-webkit-
data to this position-o-
prefix dataAlso, while we're at it, it would be nice to see the indentation fixed to align with the other data in this file (the
"opera"
line seems to have started the misalignment). This isn't from your change, just an existing quirk.I realize this feedback is a bit bigger than you probably had in mind, @antross! If you're keen to push a new commit or two to make these changes, I think it would be a big help! Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely happy to make the additional changes here and thanks @ddbeck for the detailed explanation! I'll try to update the PR later today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ddbeck I'm not sure I see the misalignment you referred to. The data immediately prior to
"opera"
for"ie"
is at a slightly different indent, but that's because it's composed of a single version entry as opposed to an array containing multiple entries.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@antross Sorry, I looked at the actual file on my machine and the indentation is fine. I think it was a quirk of how GitHub shows the expanded context. My mistake!