search

guardian / scribe-plugin-curly-quotes

5 stars 5 forks source link

Remove Scribe Common #15

Closed rrees closed 9 years ago

rrees commented 10 years ago

Tests pass and the dependency on Scribe Common has been removed.

I also bumped the plumber version to avoid the clashing versions I have been suffering from.

theefer commented 10 years ago

Do you want to make the version of Scribe the plugin needs explicit? Would be good to trial that we can do this somehow.

I see it's already a devDependency of the bower.json of the main branch, but it points to scribe#master, which isn't really descriptive enough to help detect and resolve conflicts.

It'd also need to be in the bower.json of the dist branch as that's what's read by Bower when you bower install the plugin.

rrees commented 10 years ago

@theefer Updating the dependency seemed to kill the CI build so I have raised #16 for this.

hmgibson23 commented 10 years ago

Don't know if this is related to the master stuff but it seems helpful https://github.com/guardian/scribe-plugin-smart-lists/pull/10

rrees commented 9 years ago

Now uses Immutable to replace Lodash in the plugin.

Referencing: https://github.com/guardian/scribe/issues/291

theefer commented 9 years ago

@hmgibson it is indeed exactly that

theefer commented 9 years ago

@hmgibson23 even ^

theefer commented 9 years ago

Worth noting that exposing Immutable on scribe like so causes it to be unversioned, or rather transitively version through the version of Scribe (and the version of Immutable that it depends on). This makes it rather hard to track and impossible to override the Immutable version (say to a patch or minor upgrade that includes a fix this plugin needs).

This is something I've learnt in practice doing something similar with plumber and Rx. I'd urge you to think about it before applying this to all plugin...

The best fit for this would be peerDependencies (npm), but as conjectured in the previous discussions with @OliverJAsh, it should also work using dependencies in the bower.json. Both would make the package manager share the version it downloads, picking the latest that satisfies all dependency constraints, and forcing the user to resolve any conflict.

Note that it's the exact same thing as the dependency on Scribe, so nothing new or more complicated.

rrees commented 9 years ago

Due to changes in master I'm going to kill this and resubmit.