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

Upgrade collection2 and SimpleSchema #126

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hashcutdev
Copy link

Fixes #124

Upgraded aldeed:collection2-core and new NPM version of simpl-schema as the old versions of aldeed:collection2 and aldeed:simple-schema are deprecated.

New pull request includes NPM dependency on the new package simpl-schema.

@matteodem
Copy link
Contributor

Do you have any idea why the Travis CI build is failing? I see the Npm.depends statement but it for some reason doesn't seem to run through.

@hashcutdev
Copy link
Author

Sorry, I didn't see your message until today.
I tried to figure it out but couldn't see why.
It builds on my machine (usual excuse :-)
Looking through the travis logs, it seems that travis doesn't pull in the simpl-schema npm package, even though it's included in the dependencies.

From the logs:
arkham:comments-ui: updating npm dependencies -- linkifyjs, simpl-schema...

So it sees the package dependency. But then it says:
WARNING: npm peer requirements (for aldeed:meteor-collection2-core) not installed:
[email protected] not installed.

I'm not sure why it's not installing simpl-schema even though it's listed as an npm dependency...

I tested it on my machine, including deliberately removing simpl-schema from my overall project dependency to see if the comments-ui package would install it and it did. I'm not sure why it's not working in your Travis build.

@wei170
Copy link

wei170 commented Sep 1, 2017

Is anyone still working on this pull request?

@hashcutdev Thank you for this upgrade.

@matteodem
Copy link
Contributor

@hashcutdev could you have a look at my code review feedback?

@vjrj
Copy link
Contributor

vjrj commented Jan 17, 2018

Hi,

Maybe it's related or maybe it's not, but I was testing this PR in my project (meteor 1.6, collection2, npm-simpl-schema and react) but it caused the client code to crash badly (too many unhelpful errors in console like: meteor/meteor#8693).

Adding the new dependencies one by one I detected that the crash cause was the use of moment meteor package (because and I'm already using moment npm package ^2.19.1). The moment meteor package is 2.12 (not updated in the last two years).

After switching your package locally to use momentjs via npm (minor change), all work as expected.

The changes are minimal (some moment imports and add the moment dependency).

Maybe this help others,

@vjrj
Copy link
Contributor

vjrj commented Feb 15, 2018

I'm already testing this PR locally. Other issue (an old reference to simple-schema):

diff --git a/versions.json b/versions.json
index 807ba8a..38d7c0b 100644
--- a/versions.json
+++ b/versions.json
@@ -5,10 +5,6 @@
       "2.2.0"
     ],
     [
-      "aldeed:simple-schema",
-      "1.2.0"
-    ],
-    [
       "application-configuration",
       "1.0.3"
     ],

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.

4 participants