Multi-packages split

[A-312]

fix: #321
fix: #317

validator and moment is no longer a dependencie of convict

WARNING :

Now, you should publish with lerna publish instead of npm publish

[coveralls]

Coverage remained the same at 100.0% when pulling ec2e49e on A-312:nodep into 015aead on mozilla:feat-multi-packages-split.

[A-312]

@A-312 in this PR there are many many files that have been changed (135). Obviously many shouldn't have been. Probably problems happened on your rebase. Maybe could you create a totally new and clean PR based on current master? Thank you.

This is my PR, see :

• https://github.com/A-312/node-convict/blob/nodep/README.md
• https://github.com/A-312/node-convict/tree/nodep/packages

EDIT :

• And see my first commit : cf4a41f

[A-312]

135 files change because : (I will try to count files changed)

• First commit, I move config files (9 files), ./lib/ and /test/ to ./packages/convict/. test files is about ~90 files ;
• After, I made package : convict-moment and convict-moment with their own test, readme, config and js file (14 files = 7 files x 2) ;

= 113 files

• Edits exemples (2 files), readme, config (9 files).

= 124 files

124 is near of 135, that seem good.

[A-312]

@vladikoff Do you have news about @madarche ? No activity since 2 months :( I Just need to know if I need to make my change on my repos or here.

[A-312]

@mozilla Sorry, but i don't have any news of a maintener of this repository (for my several PR since 2 months).

[vladikoff]

I shall take a look at this during the week

[madarche]

Hello all, I'm sorry for the absence. I've been trying hard to to devote some time to convict all this time. I'm back for as much as I can.

[madarche]

First, I'm all for the proposed change: This goes in the right direction for making convict simpler and easier to maintain and contribute to. And we should release convict@6 when this PR is merged.

Here are some remarks:

• I have created a new branch feat-multi-packages-split. Could you rework this PR to make it against this branch please? That way we could work together in the branch.

• Could you rework this PR, because it doesn't include at least some parts of the commits c90e7e9 and 0cbc321

• Don't copy/put .eslintrc and .eslintignore files in every packages, as all the eslint scripts will use the files at the root just fine.

• The convict-validator package name may not be very well chosen, as it makes think that the validation functionality of convict is provided by this package. What would you think about the name convict-format-email-ipaddress-url. Yes it's long but it's all very clear. Likewise convict-moment should then be renamed into convict-format-duration-timestamp. What do you think?

[A-312]

CHECKLIST : in progress.

• I have created a new branch feat-multi-packages-split. Could you rework this PR to make it against this branch please? That way we could work together in the branch.
• Could rework this PR, because it doesn't include at least some parts of the commits c90e7e9 and 0cbc321
• Don't copy/put .eslintrc and .eslintignore files in every packages, as all the eslint scripts will use the files at the root just fine.
• The convict-validator package name may not be very well chosen, as it makes think that the validation functionality of convict is provided by this package. What would you think about the name convict-format-email-ipaddress-url. Yes it's long but it's all very clear. Likewise convict-moment should then be renamed into convict-format-duration-timestamp. What do you think?

[A-312]

Don't copy/put .eslintrc and .eslintignore files in every packages, as all the eslint scripts will use the files at the root just fine.

These files will be missing if we download the package with npm and try to run lint but I think is not usefull to run eslint in node_module instead of github folder.

[madarche]

These files will be missing if we download the package with npm and try to run lint but I think is not usefull to run eslint in node_module instead of github folder.

You are right, those files will be missing if those packages are downloaded from the NPM registry to be tested! We, developers of Convict, are always working from the Git repository, but that might be useful for someone evaluating the convict packages from node_modules, and we can dream, in the future the NPM registry could automatically launch tests on a publish request to better assert the security of a package.

So I withdraw this remark. Sorry! And again thanks for the good insight.

[A-312]

The convict-validator package name may not be very well chosen, as it makes think that the validation functionality of convict is provided by this package. What would you think about the name convict-format-email-ipaddress-url. Yes it's long but it's all very clear. Likewise convict-moment should then be renamed into convict-format-duration-timestamp. What do you think?

I agree with that convict-validator and convict-format was temporary. But what do you think of :

• "convict-format-with-moment"
• "convict-format-with-validator" or "convict-format-validator-support"

I don't think if we have several package in one we should add each format name.

Because some people can extend/add format in validator package. Or maybe do something dynamically like that :

convict.addFormats(require("convict-format-with-validator")({
   "currency": "isCurrency",
   "email": "isEmail"
}));
// or : 
convict.addFormats(require("convict-format-with-validator")([ "currency", "email" ]));
// a function will add first uppercase and "is" to convert the format as validator function "currency" => "isCurrency"

[madarche]

I'm 100% convinced! 😀

As for the names, I like convict-format-with-moment and convict-format-with-validator the most.

[A-312]

@madarche done :D

[A-312]

@madarche done

[madarche]

@A-312 this is all very good, thanks! So I'm merging your work into feat-multi-packages-split.

In the next following days I'll be doing some very little edits in this branch and I'll be using this branch on some projects, before finally merging into master. That will be convict@6 then 😁