Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor code structure adjustments to the bundles.js file #15079

Merged
merged 5 commits into from
Apr 25, 2019

Conversation

kunukn
Copy link
Contributor

@kunukn kunukn commented Mar 10, 2019

Purpose:
Consistent code in build.js and bundles.js.

build.js destructured bundleTypes, I think bundles.js should too.

yarn prettier on the bundles.js moved stuff around.

Added deepfreeze for bundleTypes and moduleTypes

Before submitting a pull request, please make sure the following is done:

  1. Fork the repository and create your branch from master.
  2. Run yarn in the repository root.
  3. If you've fixed a bug or added code that should be tested, add tests!
  4. Ensure the test suite passes (yarn test). Tip: yarn test --watch TestName is helpful in development.
  5. Run yarn test-prod to test in the production environment. It supports the same options as yarn test.
  6. If you need a debugger, run yarn debug-test --watch TestName, open chrome://inspect, and press "Inspect".
  7. Format your code with prettier (yarn prettier).
  8. Make sure your code lints (yarn lint). Tip: yarn linc to only check changed files.
  9. Run the Flow typechecks (yarn flow).
  10. If you haven't already, complete the CLA.

Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html

@@ -434,19 +434,16 @@ const bundles = [

/******* ESLint Plugin for Hooks (proposal) *******/
{
Copy link

Choose a reason for hiding this comment

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

any reason for this change? looks like it was intended to fit under the
/******* ESLint Plugin for Hooks (proposal) *******/ title

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not from me.
I executed yarn prettier as peer pull request guidance and prettier decided to do that.

@ahtee
Copy link

ahtee commented Mar 11, 2019

looks great. does this fix any issue opened? can get it merged faster if so

@kunukn
Copy link
Contributor Author

kunukn commented Mar 11, 2019

looks great. does this fix any issue opened? can get it merged faster if so

I don't think so. It was something I stumbled on as I looked around in the source code.

@ahtee
Copy link

ahtee commented Mar 12, 2019

I don't think so. It was something I stumbled on as I looked around in the source code.

Ok it'll be up to maintainers obviously if this will get merged then. I suppose the tests ran ok as well when you ran them?

@kunukn
Copy link
Contributor Author

kunukn commented Mar 12, 2019

Yes the test ran ok

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

@cpojer cpojer merged commit c530639 into facebook:master Apr 25, 2019
@kunukn kunukn deleted the feature/minor-refactor-build-file branch April 25, 2019 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants