Closed danielgindi closed 2 years ago
@danielgindi - Thanks again! A few notes here:
require('cronstrue/dist/i18n/locales/fr')
? Should we make use of exports
in package.json or copy entry files to root so that consumers can do this: require('cronstrue/fr')
?var cronstrue = require('cronstrue/dist/i18n/locales/fr');
console.log(cronstrue.toString("*/5 * * * *"));
outputs:
[object Module]
@bradymholt
- I ran the tests here and am seeing some failures and I also see the same failures locally. Do you know what is going on there?
I just fixed it and pushed. Lot's of typescript issues. Nothing functional of course as I did not originally touch the source code.
- Would you mind updating the README here, related to i18n?
Just did :-)
- How do you imagine these locales will be imported? With something like
require('cronstrue/dist/i18n/locales/fr')
? Should we make use ofexports
in package.json or copy entry files to root so that consumers can do this:require('cronstrue/fr')
?
It's in the readme now...
Anyway I've updated it to generate the locale files to /locales
so it will be easier to reference.
I've also took the liberty to clean the repo from dist file completely, and only generate the dist files on prepublish
(when you do an npm publish
).
- When testing locally, I'm having trouble requiring with CommonJS. You should first require the main
cronstrue
(without locales), and then require the individual locales you want. Like this:
var cronstrue = require('cronstrue');
require('cronstrue/locales/fr');
console.log(cronstrue.toString("*/5 * * * *"));
@bradymholt gentle ping :)
Thanks!
I'm making a few tweaks to these changes in https://github.com/bradymholt/cRonstrue/pull/230 and once I'm done with that I will merge and publish.
Released as part of 2.1.0.
Hi @danielgindi - this is great! I've been really hoping someone would make this contribution. I am tied up through the end of this week but have a reminder to take a look here early next week.