phantomas
phantomas copied to clipboard
macbre
Slimmer Phantomas with a plugin/extension model
This is just a simple suggestion but I think it could be good to adopt an "extension model" for reporters and engines.
Concretely, I'm suggesting to split the code base and keep the essentials here as the project will no doubt continue to grow both in code size and popularity. The idea would be to extract most of reporters in different npm modules just like grunt or gulp did (default would be json). Same goes for engines (default would be webkit).
Why? Simple answer : In some cases I need a complexe reporter and in some cases I don't. Like most of modern developer, I like to keep my number of external dependencies as small as possible. Moreover, this change will drastically decrease install time.
Tell me what you think.
@macbre It makes sens to you ?
If yes, I can start to design something on my free time. If not, I won't push any further
I guess we've reached the milestone where phantomas can be considered quite a large and complex project. And it's worth to refactor it into smaller components.
@EFF, your suggestion does make sense. Let's discuss the details.
We can start with extracting custom reporters into separate npm modules and just leave plain and json ones in the core.
I can start with phantomas-reporter-elasticsearch.
Sorry I was pretty busy lately. I should be able to tackle a couple of reporters this weekend as well. Please make sure that you specify every you do on this so we don't do the job twice.
More over, what was your plan ? simply require the write "phantomas-extension" when and if not installed to throw a basic error like "please make sure to install
I'd replace the import of a "local" reporter in reporter.js with a require("phantomas-reporter-<reporter name>"). And of course update the error message.
@macbre what do you think if we do it gradually. What I mean by that is while we migrate to this model, we should have both "local" and "external" reporters. Concretely we could add the require("phantomas-reporter-<reporterName>") in a try catch block, if the reporter is not installed as an "external" we try the old "local" import. Then, once we're done, we remove the local reporter import (or not if you want to stay extensible)
@macbre where do you want to extract the reporters ? Do you want to create a phantomas organization in order to keep most of phantomas development at the same place on GitHub ?
Development of phantomas reporters in "private" repositories (i.e. not under github.com/macbre or github.com/phantomas) is fine to me. npm will be used to install reporters. As long as we keep using phantomas in the package names and keywords section in package.json we should be fine. Let's see how it goes :)
@macbre let me ask github customer support , the account appears to be empty and inactive. If it is in fact, they might give it away.
After running the following I'm even more for it :)
macbre@debian:~/github/phantomas$ du -hs node_modules/
202M node_modules/
@macbre I wrote to GitHub, the phantomas account is not qualified as 'dormant' in their records but they contacted the person for me just in case.
see : https://github.com/EFF/phantomas-reporter-cloudwatch for cloudwatch reporter.
I'll continue with statsd another day
@EFF thanks x3!
As for the phantomas-reporter-cloudwatch: the package is on npm, I guess we can now remove this reporter (and its dependency) from the main phantomas repo.
@EFF, I might have misunderstood the point of this ticket, but shouldn't we exclude phantomas-reporter-cloudwatch package from package.json? If someone wants to report to cloudwatch, then just run npm install phantomas-reporter-cloudwatch. Otherwise the size of all dependencies needed for phantomas to be installed will stay the same.