Fixes #148 - Extra arg on Kinto#collection to replace Collection#use. #149

michielbdejong · 2015-09-07T13:16:22Z

No description provided.

michielbdejong · 2015-09-07T13:36:21Z

@n1k0 maybe you prefer:

coll.configure({
  transformers: {
    remote: [ myRemoteTransformer ]
  }
});

instead of:

coll.configure({
  remoteTransformers: [ myRemoteTransformer ]
});

n1k0 · 2015-09-07T15:15:58Z

The second version is fine, the first is quite verbose and not much more explicit.

n1k0 · 2015-09-07T16:35:27Z

src/collection.js

+    }
+    if (config.remoteTransformers instanceof Array) {
+      this._transformers.remote = config.remoteTransformers;
+    }


We should probably raise instead of silently ignoring the passed value here.

michielbdejong · 2015-09-08T11:05:19Z

I went for this._plugins in the end. Hope you like it! Let me know if test coverage is ok, as well.

r? @n1k0

n1k0 · 2015-09-08T11:07:38Z

docs/api.md

@@ -412,7 +412,7 @@ Transformers are basically hooks for encoding and decoding records, which can wo

 ### Remote transformers

-Remote transformers aim at encoding records before pushing them to the remote server, and decoding them back when pulling changes. Remote transformers are registered by calling the `Collection#use()` method, which accepts a `Kinto.transformers.RemoteTransformer`-derived object instance:
+Remote transformers aim at encoding records before pushing them to the remote server, and decoding them back when pulling changes. Remote transformers are registered through the optional second argument of `Kinto#collection()`, which accepts `Kinto.transformers.RemoteTransformer`-derived object instances in its remoteTransformers array.


Nit: remoteTransformers (backticks)

n1k0 · 2015-09-08T11:31:53Z

This is shaping good, I like the way we explicitly declare the transformers we plan to use for a given collection as soon as we retrieve an instance of it; some small concerns with the tests which are deviating a little from the general adopted strategy. Hope my comments help here, though feel free to reach out for help if needed :)

Thanks a bunch!

michielbdejong · 2015-09-08T13:54:39Z

Thanks for the detailed feedback, will improve the structure of the tests!

michielbdejong · 2015-09-10T11:52:12Z

Coverage increased (+0.4%) to 100.0% \o/ :)

michielbdejong · 2015-09-10T11:56:11Z

@n1k0 wrote:

Wow, super nice point. We should stop caching collection instances methinks.

OK, that would simplify the code further. I think most apps will cache their constructed collections anyway (at least the Sync app does).

michielbdejong · 2015-09-14T08:05:29Z

r? @n1k0

n1k0 · 2015-09-14T08:10:35Z

docs/api.md

@@ -412,7 +412,7 @@ Transformers are basically hooks for encoding and decoding records, which can wo

 ### Remote transformers

-Remote transformers aim at encoding records before pushing them to the remote server, and decoding them back when pulling changes. Remote transformers are registered by calling the `Collection#use()` method, which accepts a `Kinto.transformers.RemoteTransformer`-derived object instance:
+Remote transformers aim at encoding records before pushing them to the remote server, and decoding them back when pulling changes. Remote transformers are registered through the optional second argument of `Kinto#collection()`, which accepts `Kinto.transformers.RemoteTransformer`-derived object instances in its `remoteTransformers` array.


We should probably add a first-level Plugins heading and describe their underlying concept.

Scratch this comment; as per IRC discussion, options would be more consistent.

n1k0 · 2015-09-14T08:21:30Z

@michielbdejong r=r+wc

michielbdejong · 2015-09-14T10:24:06Z

Rebased and squashed.

Fixes #148 - Extra arg on Kinto#collection to replace Collection#use.

michielbdejong · 2015-09-14T10:46:48Z

Thanks for all your help with this!!

…#use.

michielbdejong pushed a commit to michielbdejong/kinto.js that referenced this pull request Sep 7, 2015

Adapt tests and docs to API changes from Kinto#149

d19d1ae

michielbdejong changed the title ~~Introduce to replace (breaking change), fix #148~~ Introduce Collection#configure to replace Collection#use (breaking change), fix #148 Sep 7, 2015

michielbdejong force-pushed the 148-collection-configure branch from c2473af to 522a340 Compare September 7, 2015 13:32

michielbdejong force-pushed the 148-collection-configure branch from 522a340 to 9c0d43f Compare September 7, 2015 13:50

n1k0 reviewed Sep 7, 2015
View reviewed changes

michielbdejong mentioned this pull request Sep 7, 2015

Move Collection#use to optional second argument of Kinto#collection #148

Closed

michielbdejong force-pushed the 148-collection-configure branch from 9c0d43f to 16f7fd0 Compare September 8, 2015 10:11

michielbdejong changed the title ~~Introduce Collection#configure to replace Collection#use (breaking change), fix #148~~ Move Collection#use to optional second argument of Kinto#collection (breaking change), fix #148 Sep 8, 2015

n1k0 reviewed Sep 8, 2015
View reviewed changes

michielbdejong mentioned this pull request Sep 9, 2015

Avoid typing and extending classes for transformers #155

Closed

michielbdejong force-pushed the 148-collection-configure branch 2 times, most recently from 8571fad to 5799e68 Compare September 10, 2015 11:49

michielbdejong force-pushed the 148-collection-configure branch from b62020e to 3d19585 Compare September 11, 2015 08:16

n1k0 reviewed Sep 14, 2015
View reviewed changes

michielbdejong force-pushed the 148-collection-configure branch from 9c06f4e to dbfdd58 Compare September 14, 2015 10:23

michielbdejong force-pushed the 148-collection-configure branch from dbfdd58 to 3468b0e Compare September 14, 2015 10:45

michielbdejong changed the title ~~Move Collection#use to optional second argument of Kinto#collection (breaking change), fix #148~~ Fixes #148 - Extra arg on Kinto#collection to replace Collection#use. Sep 14, 2015

michielbdejong added a commit that referenced this pull request Sep 14, 2015

Merge pull request #149 from michielbdejong/148-collection-configure

2775503

Fixes #148 - Extra arg on Kinto#collection to replace Collection#use.

michielbdejong merged commit 2775503 into Kinto:master Sep 14, 2015

michielbdejong deleted the 148-collection-configure branch September 14, 2015 10:46

Fixes Kinto#148 - Extra arg on Kinto#collection to replace Collection…

3468b0e

…#use.

n1k0 mentioned this pull request Sep 29, 2015

Bump 1.0.0-rc.4. #187

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #148 - Extra arg on Kinto#collection to replace Collection#use. #149

Fixes #148 - Extra arg on Kinto#collection to replace Collection#use. #149

michielbdejong commented Sep 7, 2015

michielbdejong commented Sep 7, 2015

n1k0 commented Sep 7, 2015

n1k0 Sep 7, 2015

michielbdejong commented Sep 8, 2015

n1k0 Sep 8, 2015

n1k0 commented Sep 8, 2015

michielbdejong commented Sep 8, 2015

michielbdejong commented Sep 10, 2015

michielbdejong commented Sep 10, 2015

michielbdejong commented Sep 14, 2015

n1k0 Sep 14, 2015

n1k0 Sep 14, 2015

n1k0 commented Sep 14, 2015

michielbdejong commented Sep 14, 2015

michielbdejong commented Sep 14, 2015

Fixes #148 - Extra arg on Kinto#collection to replace Collection#use. #149

Fixes #148 - Extra arg on Kinto#collection to replace Collection#use. #149

Conversation

michielbdejong commented Sep 7, 2015

michielbdejong commented Sep 7, 2015

n1k0 commented Sep 7, 2015

n1k0 Sep 7, 2015

Choose a reason for hiding this comment

michielbdejong commented Sep 8, 2015

n1k0 Sep 8, 2015

Choose a reason for hiding this comment

n1k0 commented Sep 8, 2015

michielbdejong commented Sep 8, 2015

michielbdejong commented Sep 10, 2015

michielbdejong commented Sep 10, 2015

michielbdejong commented Sep 14, 2015

n1k0 Sep 14, 2015

Choose a reason for hiding this comment

n1k0 Sep 14, 2015

Choose a reason for hiding this comment

n1k0 commented Sep 14, 2015

michielbdejong commented Sep 14, 2015

michielbdejong commented Sep 14, 2015