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] Consolidate review notes #263

Closed
VWoeltjen opened this issue Nov 6, 2015 · 4 comments
Closed

[API] Consolidate review notes #263

VWoeltjen opened this issue Nov 6, 2015 · 4 comments
Assignees
Milestone

Comments

@VWoeltjen
Copy link
Contributor

Consolidate review notes on proposed API changes, #69, from #261 and #262. Proposed changes which have consensus should be marked as accepted in some fashion. If there are any gaps in original feedback points to address after acceptance, schedule a working session with development to reconcile these points.

Goal of this task is to create a set of changes which the development team agrees will address various feedback thus far, to facilitate subsequent planning.

@VWoeltjen VWoeltjen self-assigned this Nov 6, 2015
@VWoeltjen VWoeltjen added this to the Banks milestone Nov 6, 2015
@VWoeltjen
Copy link
Contributor Author

Proposal @VWoeltjen @larkin @akhenry Consensus
RequireJS as dependency injector 👎 😐 ❓ 👎
Arbitrary HTML Views 👍 👍 👍 1
Wrap Angular Services 👎 👎 👎 🚫
Bundle Declarations in JavaScript 👍 😐 ❓ 👍
Pass around a dependency injector 👎 👎 👎
Remove partial constructors 👍 👍 👍
Rename Views to Applications 👍 😐 ❓ 👍 2
Provide Classes for Extensions 👍 👍 👍
Normalize naming conventions 👍 👍 👍
Expose no third-party APIs 👍 * 👎 👍 👍 3
Register Extensions as Instances instead of Constructors 👍 👎 👍
Remove capability delegation 👍 👍 👍
Nomenclature Change 👍 👍 ✅ ‡
Capabilities as Mixins 👍 👍 4
Remove Applies-To Methods 👎 👎
Revise Telemetry API 👍 👍 👍 5
Allow Composite Services to Fail Gracefully 👍 👎 👍 6
Plugins as Angular Modules 👍 😐 ❓
Contextual Injection 👎
Add new abstractions for actions 👎 👍 👎
Add gesture handlers 👍 👍 ❓ 👍

* Excepting Angular APIs. Internally, continue to use code style where classes are declared separately from their registration, such that ubiquity of Angular dependency is minimized.
† "I think we should limit the third party APIs we expose to one or two, but I worry it might be counterproductive to completely hide them."
‡ Some ambiguity about what to call ourselves if not a platform, but general agreement that "platform" is not a good term.
More Detail on Pete's Opinions Here: https://github.com/nasa/openmctweb/blob/api-redesign/docs/src/design/proposals/APIRedesign_PeteRichards.md#notes-on-current-api-proposals

1 Needs to be designed carefully; don't want to do this with a complicated interface, needs to be significantly simpler than wrapping with an Angular directive would be.
2 Agree that we need a new name, but it should not be "application"
3 Don't want to expose (or require usage of) third-party APIs generally. Angular may be an exception in the sense that it is an API we presume to be present. Can use third-party APIs internally, but don't want to support them or be tied to them.
4 Want to have a separate spin-off discussion about capabilities. Want to consider several alternatives here. At minimum, though, mixins would be an improvement relative to how these are currently handled.
5 Agree we want to revise APIs, but this should be a larger spin-off.
6 Not necessarily as described, but expected to be a property of composite services in whatever formulation they take. Should not be default behavior.

@VWoeltjen
Copy link
Contributor Author

Moving to Baxter; per comments in #261 more time is needed for reviews. Scheduled meeting for Dec. 8 to follow up.

@VWoeltjen VWoeltjen modified the milestones: Baxter, Banks Nov 25, 2015
@VWoeltjen
Copy link
Contributor Author

Proposal Consensus
Imperitive component registries 👍
Get rid of "extension category" concept. 👍
Reduce number and depth of extension points 👍
Composite services should not be the default
Get rid of views, representations, and templates. 👍 1
More angular: for all services
Less angular: only for views
Use systemjs for module loading 👍 2
Use gulp or grunt for standard tooling 👍
Package openmctweb as single versioned file. 👍
Refresh on navigation 👍 3
Move persistence adapter to promise rejection. 👍
Remove bulk requests from providers 👍 4

1 Need to agree upon details at design-time, but basic premise is agreed-upon - want to replace views/representations/templates with a common abstraction (and hoist out the non-commonalities to other places as appropriate)
2 Beneficial but not strictly necessary (may be lower-effort alternatives); should prioritize accordingly during planning
3 Some effort will be required to make all of the state that needs to persist among route changes actually be persistent. Will want to address this at design-time (will want to look at libraries to simplify this, for instance)
4 Maybe not all providers, but anywhere there is not a strong case for building batching into the API we should prefer simplicity. (Want to pay specific attention to telemetry here.)

VWoeltjen added a commit that referenced this issue Jan 6, 2016
Bring over tables indicating consensus/decisions about
proposed API changes from
#263
@VWoeltjen
Copy link
Contributor Author

Decisions above merged in with #69. Remaining work allocated to issues as linked-to from the tables in preceding comments (also included at conclusion of API Redesign Proposals)

This was referenced Jan 6, 2016
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

No branches or pull requests

1 participant