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

[API] API Redesign Goals and Proposals #69

Merged
merged 37 commits into from
Jan 6, 2016
Merged

[API] API Redesign Goals and Proposals #69

merged 37 commits into from
Jan 6, 2016

Conversation

VWoeltjen
Copy link
Contributor

Creating a pull request to allow comments in-line.

@VWoeltjen VWoeltjen changed the title [API] API Redesign Goals [API] API Redesign Goals and Proposals Aug 27, 2015
@VWoeltjen VWoeltjen mentioned this pull request Sep 8, 2015
Add note about RequireJS as a dependency injector
facilitating use from web workers, as noted in
comments to #141

## Get rid of "extension category" concept.

The concept of an "extension category" is itself an extraneous concept-- an extra layer of interface depth, an extra thing to learn before you can say "hello world". Extension points should be clearly supported and documented with whatever interfaces make sense. Developers who wish to add something that is conceptually equivalent to an extension category can do so directly, in the manner that suites their needs, without us forcing a common method on them.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I agree with this.

What I want to avoid is providing multiple interfaces for the same thing. For instance (assuming the "register during config" approach):

mct.config(['fooProvider', 'barProvider', function (fooProvider, barProvider) {
    fooProvider.registerFoo(myFoo);
    barProvider.bars.push(myBar);
}]);

...would be bad (increase number of interfaces needlessly.) We should have/use a common RegistryProvider.<T> interface for these (and, ideally, some implementation(s) that can be reused if plugins wish to introduce extension points):

mct.config(['fooProvider', 'barProvider', function (fooProvider, barProvider) {
    fooProvider.register(myFoo);
    barProvider.register(myBar);
}]);

This fills the same role as extension categories, so I'm not sure if that would contradict this proposal or not. I agree that we shouldn't force a common method, but we shouldn't prevent using a common method either.

Copy link
Contributor

Choose a reason for hiding this comment

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

This proposal is a "policy statement" about how we communicate these concepts to developers and users more so than a specific requirement for code.

Yes, Registering providers should have consistent syntax, but that's just good programming; it's not a feature.

The developer guide should show how to extend the platform while introducing the bare minimum number of concepts. "RegistryProvider" is a concept that they just don't need to know about.

@VWoeltjen VWoeltjen mentioned this pull request Dec 24, 2015
Particularly, closing API-related issues; need for this
mentioned at
#264 (comment)
Bring over tables indicating consensus/decisions about
proposed API changes from
#263
@VWoeltjen
Copy link
Contributor Author

Added decisions from #263 to proposals, as well as plans from #264.

Would like to merge into master at this point to formalize decisions/plans. All outstanding work (both pending decisions and plans) is linked to open issues.

Author Checklist

  1. Changes address original issue? Y ([API] Consolidate review notes #263 and [API] Plan refactoring #264)
  2. Unit tests included and/or updated with changes? N/A (changes are documentation-only)
  3. Command line build passes? Y
  4. Changes have been smoke-tested? Y

@VWoeltjen VWoeltjen assigned akhenry and unassigned VWoeltjen Jan 6, 2016
@VWoeltjen VWoeltjen modified the milestones: Bradbury, Baxter Jan 6, 2016
@akhenry
Copy link
Contributor

akhenry commented Jan 6, 2016

Reviewer Checklist

  1. Changes appear to address issue? Y
  2. Appropriate unit tests included? N/A
  3. Code style and in-line documentation are appropriate? N/A
  4. Commit messages meet standards? Y

akhenry added a commit that referenced this pull request Jan 6, 2016
[API] API Redesign Goals and Proposals
@akhenry akhenry merged commit 02c984a into master Jan 6, 2016
@akhenry akhenry deleted the api-redesign branch January 6, 2016 19:21
@VWoeltjen VWoeltjen mentioned this pull request Jan 28, 2016
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants