Use webpack to build distributions

[musicin3d]

This is how I propose we address this comment: https://github.com/chrismbryant/amazon-confidence-interval/issues/9#issuecomment-602373312

With webpack in place, we can simply install dependencies with npm or yarn and import them in the source code. Dependencies will not be stored in the repo; they will be installed on the dev machine from online repos. Webpack will automatically bundle the dependencies as needed and optimize all source code.

Readme was updated with instructions for using the new build system.

[chrismbryant]

Nice work! Now suppose I include stdlib as a dependency in package.json:

"dependencies": {
  "@stdlib/stdlib": "^0.0.32"
}

When I run npm install, that installs the dependency onto my machine, but I can't figure out how to actually import the module I need so that confidence_interval.js can access it. What needs to be done to get "@stdlib/stdlib/dist/stdlib-flat.min.js" bundled into the build?

[musicin3d]

Their readme includes the following example: const dswap = require('@stdlib/blas/base/dswap') (edited for best practices)

You should also be able to use the more modern syntax: import {blas} from 'stdlib' On second look, it's not supported.

I can look into this tomorrow end of today.

[aeciorc]

Their readme includes the following example: const dswap = require('@stdlib/blas/base/dswap') (edited for best practices)

~You should also be able to use the more modern syntax: import {blas} from 'stdlib'~ On second look, it's not supported.

I can look into this ~tomorrow~ end of today.

I gave this a shot. Importing with require worked. Since we can also import confidence_interval from inject.js, we can remove the content_scripts part of manifest.json, since everything will already be bundled in inject.js. We also don't need to copy js files to dist/shared anymore since they won't be used.

Note: I had to add

  node: {
      fs: "empty"
    },

to the webpack config, otherwise I got errors when requiring stdlib

[musicin3d]

I went ahead and updated the code to fully utilize the packaging system.

• Added @stdlib/stdlib as a dependency
• Included the stats distribution of stdlib
• Refactored confidence_interval to be a module, named calculations
• Added a dump of the confidence interval on each product
• Fixed a few minor things like formatting and inspection warnings

I didn't get any errors by including stdlib. I used const stdlib = require('@stdlib/stdlib/dist/stdlib-stats-base-dists-flat')

[musicin3d]

I also

• fixed Chris' test.html so it works again. :)
• implemented aeciorc's suggestion of not copying js files

It's only copying images now. This approach will need to change if we end up with lots of different things to copy, because it needs to be easy for others to maintain. But for now, this implementation works perfectly, and it's not over-engineered.

[aeciorc]

Looks really good, just made a comment about us sticking to either imports or requires. Other than that I think this PR is good to go

[chrismbryant]

This is great! I have nothing to add right now, but I'm happy to see the work progressing so quickly. Sorry I couldn't contribute more to this side of things, but you two have a far better handle on this than I do :) @musicin3d, feel free to merge this PR whenever you feel comfortable!

[musicin3d]

I don't have permission to accept the merge request.

This is what I see...

This branch has no conflicts with the base branch Only those with write access to this repository can merge pull requests.

[chrismbryant]

@musicin3d I invited you as a collaborator, so you should have permission once you accept the invitation

[musicin3d]

Ah... Sorry, I'm new to collaborating on GitHub. It looks like the invitation expired. Would you mind resending it?

[chrismbryant]

No worries! This is my first real open source project, so I'm new to it, too. I just re-sent the invitation

[musicin3d]

Huzah! Do we have any opinions on the type of merge? (commit, squash, rebase)

[chrismbryant]

I personally don’t, but maybe others have opinions or wisdom on best practices?

[musicin3d]

Ok, I'll use the default for now (regular merge commit), so I can submit another pull request that depends on this. I'm interested in @aeciorc's opinion too

[aeciorc]

Ok, I'll use the default for now (regular merge commit), so I can submit another pull request that depends on this. I'm interested in @aeciorc's opinion too

We use squash at work to keep the commit history clean, but I think using a regular merge commit is more beginner-friendly since it doesn't cause the source and target branches to diverge. So imo for this project we should always use merge commits, the exception being PRs with a really messy commit history (e.g 15 commits named "updated somefile.js"), for those we may want to squash instead.