Skip to content

Conversation

joshdmiller
Copy link

@joshdmiller joshdmiller commented Aug 23, 2016

Opening for Discussion...

Closes #8


This change is Reviewable

@joshdmiller
Copy link
Author

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions.


falcor-model.js, line 99 [r1] (raw file):

       *   - R: read-only permissions
       */
      rights: $atom( Array<Enum( A, R, W )> ),

Change enum to a disjoint union. E.g. Array<WorldRight> and type WorldRight = 'A' | 'R' | 'W';.


falcor-model.js, line 360 [r1] (raw file):

       *   - A: Analytics
       */
      status: Enum( 'O', 'W', 'P', 'A' ),

Same as above. Use disjoint union.


Comments from Reviewable

@joshdmiller
Copy link
Author

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.


falcor-model.js, line 380 [r1] (raw file):

   * Outline Nodes:
   */
  outlinesById: {

Change to outlineNodesById to avoid ambiguity.


Comments from Reviewable

@joshdmiller
Copy link
Author

Change all open property types (such as [number] and [string]) to one with a property name and type (e.g. [index : number] and [id : string]).


Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.


Comments from Reviewable

@joshdmiller
Copy link
Author

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.


falcor-model.js, line 406 [r1] (raw file):

       * Child outline nodes.
       */
      children: {

Add push and remove calls here.


Comments from Reviewable

@joshdmiller
Copy link
Author

Also change all $atom references to an Atom type.

type Atom<T> = {
  $type: 'atom',
  value: T,
};

Review status: 0 of 1 files reviewed at latest revision, 4 unresolved discussions.


falcor-model.js, line 340 [r1] (raw file):

   */
  booksById: {
    [String]: {

Add outline node creation and removal calls here.


Comments from Reviewable

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.

1 participant