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.

Sign up for GitHub

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

Jump to bottom

WIP: Decorator Implementation #17226

Closed
wants to merge 10 commits into from
Closed

WIP: Decorator Implementation #17226

wants to merge 10 commits into from

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Nov 23, 2018 •
edited
Loading

Not only is this a WIP, but the RFC still needs to be merged / accepted.

Soon! ™️

Hopefully this can be merged first: #17404
:)

Outstanding work:

  • mixin and subclassing (_super) support
  • more tests for the decorators
  • tests for the the other macros as well
  • cleanup
  • make the code simpler (rather than a sorta straight copy from ember-decorators)
  • adjust for latest decorator spec changes
  • benchmarking

@NullVoxPopuli NullVoxPopuli force-pushed the implement-decorators branch 2 times, most recently from e2ca5be to fbedc41 Compare December 7, 2018 11:57
NullVoxPopuli
NullVoxPopuli commented Dec 7, 2018
View reviewed changes
Turbo87
Turbo87 reviewed Dec 21, 2018
View reviewed changes
@jacobq
Copy link
Contributor

jacobq commented Dec 21, 2018 •
edited
Loading

Just curious, how does this relate to ember-decorators? And which RFC is it related to? (240 / ES Classes?) Nevermind, I think RFC 408 answers all.

@pzuraq
Copy link
Contributor

pzuraq commented Jan 8, 2019

Tests are invaluable @NullVoxPopuli, keep up the good work!

@pzuraq pzuraq mentioned this pull request Feb 2, 2019
Merged
2 tasks
@mike-north
Copy link
Contributor

Does #17548 supersede this PR?

@pzuraq pzuraq closed this Mar 17, 2019
@pzuraq
Copy link
Contributor

pzuraq commented Mar 17, 2019

Yes it does, closing since we merged the feature in a different PR

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

@Turbo87 Turbo87 Turbo87 left review comments

@pzuraq pzuraq pzuraq left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@NullVoxPopuli @jacobq @pzuraq @mike-north @Turbo87