Skip to content

Conversation

HoraceShmorace
Copy link
Contributor

  1. "GLOBALS" is ambiguous, particularly since its only use is to configure the React environment. I don't really see a reason to abstract this at all, as opposed to just inlining the object in the call to webpack.DefinePlugin.
  2. Clarified the comment on line 25 to indicate that the statement before it sets the build to either mode, not just prod.

Note: There should probably be a comment on line 6 to explain the need to JSON.stringify a string. I'm not sure of the reason, so I couldn't add the comment myself.

1. "GLOBALS" is ambiguous, particularly since its only use is to configure the React environment. I don't really see a reason to abstract this at all, as opposed to just inlining the object in the call to `webpack.DefinePlugin`.

2. Clarified the comment on line 25 to indicate that the statement before it sets the build to either mode, not just prod.

Note: There should probably be a comment on line 6 to explain the need to `JSON.stringify` a string. I'm not sure of the reason, so I couldn't add the comment myself.
@coveralls
Copy link

coveralls commented Jul 8, 2016

Coverage Status

Coverage remained the same at 98.906% when pulling a9be5dd on HoraceShmorace:master into a7c95c5 on coryhouse:master.

@coryhouse coryhouse merged commit eeabfbc into coryhouse:master Jul 8, 2016
@coryhouse
Copy link
Owner

Thanks for the PR!

@coryhouse
Copy link
Owner

And I just inlined the code as you suggested. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants