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

Expand and generalize guidelines for what is a partial impl #12189

Closed
wants to merge 2 commits into from

Conversation

gsnedders
Copy link
Contributor

per discussion with @ddbeck, @Elchi3 and @foolip yesterday, and on the basis of Cunningham's Law ("the best way to get the right answer on the internet is not to ask a question; it's to post the wrong answer"), here is an (almost certainly poor) attempt to document when an implementation should be considered partial

#6906 and #3904 (comment) are both (still) relevant here, and some other examples which are probably worthwhile considering are #10139, #9128, #11780.

@github-actions github-actions bot added the docs Issues or pull requests regarding the documentation of this project. label Aug 26, 2021
@gsnedders
Copy link
Contributor Author

(not bothering with the lint failure right now given I'm sure we'll have plenty of discussion anyway)

@ddbeck ddbeck self-requested a review September 6, 2021 13:43
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

I can't believe this was opened three weeks ago. Sorry for the wait, @gsnedders!

It looks like the biggest changes here are the introduction of two new points for partial implementation:

  1. If the implementation has bugs which it is likely a large percentage of web developers will encounter when attempting to use the feature, then it should be marked as a partial implementation with an appropriate note

  2. If the implementation is only supported in specific contexts (for example, if break-before is only supported for printed pages but not for other types of fragmentation, such as multi-column layout), then it should be marked as a partial implementation with an appropriate note.

The first (I'll call it "large impact" for short) sounds reasonable to me and probably captures what we've done already in some cases. Perhaps we could illustrate this with some examples of how a contributor would know whether a bug has a large impact. I don't want to be too strict about this, but something along the lines of a subject-matter expert saying that the bug is likely to have a wide impact or a bug tracker issue having many complaints from developers would qualify.

The second sounds reasonable, but it's a bit fuzzier—I don't know how I'd apply this rule, necessarily. Again, we could lean on subject-matter experts. But perhaps we could bound this a bit, by referencing the specification? As a reviewer, I'd then have something to compare against. Broadly, "specific contexts" could be anything ("On pages with a #03fcf0 background…"), but if we said a specified context, then it's more clear that I'm looking for something like "in workers" or "in paged media."

This would leave non-standard features out in the cold, though maybe that's fine—non-standard features wouldn't have a benchmark for complete implementation anyway, right?

@queengooborg
Copy link
Collaborator

Hey @gsnedders, looks like this PR may be awaiting your response!

@gsnedders
Copy link
Contributor Author

Hey @gsnedders, looks like this PR may be awaiting your response!

I think I'd been hoping for more responses, rather than just dealing with Daniel's alone, but if nobody else has an opinion…!

@github-actions
Copy link

This pull request has merge conflicts that must be resolved before we can merge this.

@github-actions
Copy link

This pull request has merge conflicts that must be resolved before it can be merged.


The simplest state to explain is “unsupported”: if a feature is entirely absent then it should be marked as unsupported (`"version_added": false`).

Distinguishing the other two is harder; but as some guidelines consider the following:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Distinguishing the other two is harder; but as some guidelines consider the following:
Distinguishing the other two is harder; but consider the following:


If a browser recognizes an API name, but the API doesn’t have any discernable behavior, use `"partial_implementation": true` instead of `"version_added": false`, as if the feature has non-standard support, rather than no support.
If a browser recognizes a feature, but the feature doesn’t have any discernable behavior, then it should be marked as partially supported (`"partial_implementation": true`), as if the feature has non-standard support, rather than no support.
Copy link
Collaborator

@foolip foolip Sep 13, 2022

Choose a reason for hiding this comment

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

Suggested change
If a browser recognizes a feature, but the feature doesn’t have any discernable behavior, then it should be marked as partially supported (`"partial_implementation": true`), as if the feature has non-standard support, rather than no support.
If a browser exposes a feature, but the feature doesn’t have any discernable behavior, then it should be marked as partially supported (`"partial_implementation": true`), as if the feature has non-standard support, rather than no support.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Discernable" with an A is actually correct as well as suggested by https://wikidiff.com/discernible/discernable -- actually I didn't know the other form existed until now. :P


See [#3904](https://github.com/mdn/browser-compat-data/pull/3904#issuecomment-484433603) for additional background (and nuance).
If the implementation has bugs which it is likely a large percentage of web developers will encounter when attempting to use the feature, then it should be marked as a partial implementation with an appropriate [note](#partial_implementation-requires-a-note).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we dare name a percentage here? How about 10%?

Suggested change
If the implementation has bugs which it is likely a large percentage of web developers will encounter when attempting to use the feature, then it should be marked as a partial implementation with an appropriate [note](#partial_implementation-requires-a-note).
If the implementation has bugs which likely affect >10% web developers when attempting to use the feature, then it should be marked as a partial implementation with an appropriate [note](#partial_implementation-requires-a-note).


## Operating system limitations imply `"partial_implementation"`
If the implementation is only supported in specific contexts (for example, if `break-before` is only supported for printed pages but not for other types of fragmentation, such as multi-column layout), then it should be marked as a partial implementation with an appropriate [note](#partial_implementation-requires-a-note).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This requires a bit more nuance. Sometimes the context itself didn't originally exist, such as service workers. We shouldn't mark things as partially supported when a new such context is added and existing features are defined to work in those contexts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, true; presumably, we want to define subfeatures when that happens ... but how do we differentiate that from stuff that new contexts get "for free", like (non-sharing-related) JavaScript builtins in new types of workers?

@github-actions github-actions bot added the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Mar 3, 2023
@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This pull request has merge conflicts that must be resolved before it can be merged.

@queengooborg
Copy link
Collaborator

This PR has been sitting around for a long time, and the data guidelines have been overhauled a bit since the creation of this PR. Should we close it?

@Elchi3
Copy link
Member

Elchi3 commented Nov 10, 2023

Yes, I think the section https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines/index.md#non-functional-defined-names-imply-partial_implementation already covers the gist of this PR. Happy to review a new PR if we need to add more nuance to the guideline, though.

@Elchi3 Elchi3 closed this Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Issues or pull requests regarding the documentation of this project. merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants