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

Tighten up [Serializable] and [Transferable] definitions #5036

Merged
merged 2 commits into from
Oct 25, 2019

Conversation

domenic
Copy link
Member

@domenic domenic commented Oct 23, 2019

  • Prohibits use on interfaces involving inheritance, since we weren't
    handling that.
  • Removes the attempt at supporting mixins. Mixins have changed, so the
    text was no longer correct, and it was never very complete anyway.

This should serve as a better foundation for #4940.


/structured-data.html ( diff )

* Prohibits use on interfaces involving inheritance, since we weren't
  handling that.
* Removes the attempt at supporting mixins. Mixins have changed, so the
  text was no longer correct, and it was never very complete anyway.
@annevk
Copy link
Member

annevk commented Oct 24, 2019

File has [Serializable]. I think the way to maybe do this is to align with how File API already tackles this. Each interface can have this, but each interface is then also responsible for the full algorithm. And if you have a lot of inheritance you can sort out deduplication yourself, until that becomes common enough that abstraction is warranted. (Currently File API duplicates, as it's fairly simple, which seems fine.)

@domenic
Copy link
Member Author

domenic commented Oct 24, 2019

Thank you for catching that. Pushed a new commit that does that. New commit message body suggestion:

* Spells out how when interitance gets involved, specs need to handle
  the whole object; we don't do any automatic
  inheritance-chain-crawling.
* Removes the attempt at supporting mixins. Mixins have changed, so the
  text was no longer correct, and it was never very complete anyway.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants