Refactor getOptions

[mattjmorrison]

Hello,

I have written a wrapper library for node-coveralls called grunt-karma-coveralls which provides glue code between grunt, karma, and node-coveralls. To accomplish this I've used the sendToCoveralls and convertLcovToCoveralls functions. One piece that I'm missing is a majority of the logic inside of getOptions. For grunt-karma-coveralls I would like to be able to reuse the contents of getOptions with the exception of the following block of code:

    // try to get filepath from the command-line
    if (process.argv[2]) {
        if (~['-v', '--verbose'].indexOf(process.argv[2])) {
            if (process.argv[3]) {
                options.filepath = process.argv[3];
            }
        } else {
            options.filepath = process.argv[2];
        }
    }

Would you be willing to extract a method that would be available for me to call that contains all of the logic in getOptions except for the part dealing with process.argv?

Thanks!

[cainus]

Very cool project.

How would you provide the filepath and verbosity level then? Some signature like getOptions(filepath, verbosity, cb) ?

[mattjmorrison]

What I'm thinking is that there will be some new method that will create and return the options object that is configured based solely on process.env. That way I can get that object and add to it based on what is configured via grunt. So, the filepath would be something configurable in the Gruntfile.

[cainus]

Grunt can't do command-line options? Everything has to be an environment var?

[mattjmorrison]

Grunt is a command line tool, but the way that it works really makes it so that you don't need to use command line arguments, in fact it would be very confusing. For example, I have a test grunt task so when I want to run my tests I can just type grunt test. Then in my Gruntfile I can configure the steps that need to occur when I run the test task. For my project dsmcode-ui the test task compiles coffeescript into javascript, compiles handlebars templates, concatenates all of those files along with some other dependencies into a single javascript file and runs the tests. This uses a series of different grunt plugins, one of which is grunt-karma-coveralls. So, having a command line argument specifically for coveralls would be very confusing because there would be no way to know which of the grunt plugins might be trying to use it. Rather than using command line arguments grunt relies on configuration placed in a Gruntfile.

Does that make sense?

[cainus]

Okay... understood. What if there was an option to use a COVERALLS_LCOV_PATH environment variable that held the file path instead?

[mattjmorrison]

I don't need the path to be set at all and I would rather not have to set an environment variable. Take a look at this code.

What I envision is changing that to look something like this:

    var gruntOptions = grunt.config('coveralls.options');
    var coverallsOptions = getBaseOptions();
    coverallsOptions.filepath = gruntOptions.filepath;

Where getBaseOptions is the middle section of the current getOptions that includes everything except for the process.argv stuff at the beginning and the callback logic at the end. Then inside of getOptions you would call getBaseOptions instead of doing var options = {} Then add the filepath onto that object after the fact.

Does that make sense? If you'd like, I can submit a PR with what I'm thinking for you to review.

[mattjmorrison]

Resolved by #23 being merged, Thanks!