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

Missing a space for error 125 #8981

Merged
merged 1 commit into from
Feb 13, 2017
Merged

Missing a space for error 125 #8981

merged 1 commit into from
Feb 13, 2017

Conversation

philraj
Copy link
Contributor

@philraj philraj commented Feb 12, 2017

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

  1. Fork the repository and create your branch from master.
  2. If you've added code that should be tested, add tests!
  3. If you've changed APIs, update the documentation.
  4. Ensure the test suite passes (npm test).
  5. Make sure your code lints (npm run lint).
  6. Run the Flow typechecks (npm run flow).
  7. If you added or removed any tests, run ./scripts/fiber/record-tests before submitting the pull request, and commit the resulting changes.
  8. If you haven't already, complete the CLA.

@aweary
Copy link
Contributor

aweary commented Feb 13, 2017

Thanks for the PR @philraj! This is a generated file that pulls out invariant messages from the codebase for production, so generally, we wouldn't want to edit this file directly. In this case, this the error was actually removed in #8176 so it shouldn't even be there in the first place. I'll look into why its not being removed.

Thanks again though!

@aweary aweary closed this Feb 13, 2017
@aweary
Copy link
Contributor

aweary commented Feb 13, 2017

Alright, so per the script's README it is not supposed to delete old errors (make sense, as we still want to be able to report the production error message for applications running older versions that still have the invariant).

I think editing it in this case is fine since the actual error itself does not exist in the source anymore, but we want to preserve it. Re-opening and merging 👍

@aweary aweary reopened this Feb 13, 2017
@aweary aweary merged commit 6367f9f into facebook:master Feb 13, 2017
@gaearon
Copy link
Collaborator

gaearon commented Feb 13, 2017

I reverted in #8994 —this is an autogenerated file. 😉

@gaearon
Copy link
Collaborator

gaearon commented Feb 13, 2017

Oh wait, I missed the part about this error being removed in the source. You're right it should be fine then.
My bad 😞

@aweary
Copy link
Contributor

aweary commented Feb 13, 2017

No worries! I thought the same when I saw this. I opened a PR to revert the revert: #8995

@philraj philraj deleted the patch-2 branch February 13, 2017 23:35
@philraj
Copy link
Contributor Author

philraj commented Feb 13, 2017

Happy to help, minor as it may be 😌. Keep up the good work on React.

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