Skip to content
This repository was archived by the owner on Jun 3, 2021. It is now read-only.

Conversation

@pattiereaves
Copy link

No description provided.

@pattiereaves pattiereaves requested a review from ostowe May 19, 2018 00:05
"node": "6.11.4",
}
},
"modules": "commonjs",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason, this is the only way I could get the tests to work?! I don't understand why it wasn't necessary for the cli.

At this point contribution guidelines are limited. This document contains information on setting up and developing locally; otherwise the primary concern for contributors is whether or not you are a member of the Alley Interactive organization (Huron's sponsor).

## Development
There are two primary comonents to Huron: the CLI and the web (browser) script. Since each of these is used for different purposes, there is a different build pipeline for each.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes in this file are correcting typos

package.json Outdated
"handlebars": "^4.0.11",
"handlebars-loader": "^1.7.0",
"html-loader": "^0.5.5",
"html-webpack-plugin": "jantimon/html-webpack-plugin#webpack-4",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@goodguyry goodguyry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inconsistency in console statements' formatting (single-line vs. multi-line) bothers me, but it's obviously non-blocking. Sweet update! 🥇

"plugins": [
"jest"
],
// Do NOT change these rules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂🙅🚫💅

},
{
test: /\.js$/,
// exclude: /node_modules/,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 We don't need to exclude node_modules for either of these? If not, I'd say I'd say kill this comment.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do, this was me debugging, I'm adding it back, good catch 😅

chalk.red(`Could not delete: ${file.name}`)
);
console.warn(
chalk.red(`Could not delete: ${file.name}`));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this still be broken out in a separate line?


if (prototype.length) {
id = prototype[0];
[id] = prototype;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥🎸

info.errors
)
);
console.error(chalk.red(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too

// Requires
/** @global */

// @todo this may be deprecated with webpack 4
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There still are some places where we are parsing for env.huron so it's not completely but we probably should look at a more webpack 4 way of approaching this.

'Webpack encountered warnings during compile: ', info.warnings
)
);
console.error(chalk.yellow('Webpack encountered warnings during compile: ', info.warnings)); // eslint-disable-line max-len
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i blame the ide

Pattie Reaves and others added 3 commits June 6, 2018 10:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants