Current coverage is 100.00%

Merging #2 into develop will not affect coverage as of 4012ab5

@@            develop      #2   diff @@
=======================================
  Files           231     233     +2
  Stmts          2954    2983    +29
  Branches        404     412     +8
  Methods           0       0       
=======================================
+ Hit            2954    2983    +29
  Partial           0       0       
  Missed            0       0       

Review entire Coverage Diff as of 4012ab5

Powered by Codecov. Updated on successful CI builds.

Build breaks because Travis tries to run the test file in the es2015 sub-folder for node versions not supporting generators. That test file should not be loaded by default. We should not include script files inside of subdirectories of test, I think.

@Planeshifter Not allowing tests in subdirectories is not a good solution, as it forces us into a particular test structure and reflects our limitations rather than anything inherent in the project structure. An alternative solution is to not have a separate file, include the tests in the main test file, and then only run generator tests provided generator support. For an example, see is-number, which uses a feature of tape which allows providing test options.

I will try this out later. However, are you sure this will work? Recall that I moved the tests to a separate file precisely because the function*() syntax for generator functions will throw a SyntaxError in older environments not supporting it, irrespective of whether the code is actually executed. A less intrusive alternative to skipping all test files in sub-directories would be to only skip those that reside in a es2015 directory.

@Planeshifter Another alternative is just to rename the file from es2015/test.js to es2015/index.js, since you are simply injecting the file only if an environment has generator support. Meaning, the test runner is not the one deciding whether the file is run, but the main test file via a require.

@Planeshifter An aside: if an environment does throw a SyntaxError even if the code is not run, that would be interesting. To my knowledge, V8 lazily compiles each function upon first invocation. This, however, cannot be generalized to all environments, which may be more or less aggressive in compiling JS source.

For example, this code block throws when executed on node version v0.10.36:

function notexecuted() {
    function* geometricSeriesGenerator( x ) {
        var exponent = 0;
        while ( true ) {
            yield Math.pow( x, exponent );
            exponent += 1;
        }
    }
}

Renaming from test.js to index.js seems to be the best solution so far. Will make the necessary changes.

@Planeshifter Yes, you are right. This is also supported in node-compat, where Node <= v0.12 throws. Would be interesting to know the step-by-step process of the compiler upon first encountering JS code. The implemented workaround is good, as it isolates the exception to where the problem arises, rather than forcing a broader generalization which affects the entire project. Thanks for updating. Will review later.

@Planeshifter a few comments:

• rebase using the current develop
• README contains some errors; e.g., tolerance (1e-16 vs eps), incorrect module names, example comments, etc
• update to current module structure; e.g., empty version, JSDoc comments, etc
• tests contain anonymous functions; this should be remedied
• tests contain spurious comments
• for various require statements, ./../lib/ should be ./../lib
• centralize default options; e.g., create a separate file which exports a default options object and then use in basic and generators. ATM, possible that defaults could be changed in one file but not the other.
• function variables should be declared and listed in descending order based on variable length. opts = {} can be initialized just prior to checking for an options argument.
• in the tests, why is the relative difference 1e-14? Is this due to accumulated rounding error? If so, should we tweak the summation algorithm to use pairwise or Kahan summation?
• put a TODO note next to each jshint exception for future removal, as we will be moving to ESLint
• currently, we sniff for a generator based on next. We should create a is-generator (or is-generator-function) module for doing a proper determination. It will be a bit heavier weight, but should become negligible as the number of iterations approaches the maximum number of terms.
• I think I prefer maxTerms and initialValue as the options property names.

Thanks for these suggested improvements!
I changed the code to follow most of them. I increased the tolerance from 1e-14 to EPS, all tests still pass. No idea why I chose a higher tolerance previously.
The only point of yours which I don't agree with is moving the default options to a separate file. Yes, it is true that when changing them, one might forget to edit both files and thus introduce errors. However, bear in mind that the JSDoc annotations have to reside in each file too, and these need an update in case of any changes, too. Having both the comments and the code in one place is a better option, I feel. Your thoughts?

Hmm, I rebased but now these pull requests contain way too many commits. Any idea why this happened? We should create new pull requests to only merge in the relevant commits.