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

Rename .jsx files to .js and vice versa #334

Closed
wants to merge 2 commits into from

Conversation

nbhargava
Copy link

@roryabraham will you please review this?

Optimistically, in making changes to the Expensify.cash desktop app, I want to avoid making webpack changes that require being able to parse JSX. expensify-common is not built or packed, so the imports that trace to it are made without any modifications. When the electron app tries to drive through the tree of pre-existing imports, it gets to a JSX file that causes problems.

Right now CONST.jsx is the main offender, but I figure it's worth switching some of the others. It is worth noting that there are still lint errors, but those seem to be independent of the changes I made. Mine just elucidate them.

Fixed Issues

N/A

Tests

  1. What unit/integration tests cover your change? What autoQA tests cover your change?

Ran npm lint and saw no issues that were newly caused by renaming.

  1. What tests did you perform that validates your changed worked?

Ran npm lint and saw no issues that were newly caused by renaming.

QA

  1. What does QA need to do to validate your changes?
  2. What areas to they need to test for regressions?

@nbhargava nbhargava requested a review from a team as a code owner February 13, 2021 17:01
@github-actions
Copy link

github-actions bot commented Feb 13, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@botify botify requested review from MariaHCD and removed request for a team February 13, 2021 17:02
@nbhargava
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@MariaHCD MariaHCD requested review from roryabraham and removed request for MariaHCD February 15, 2021 06:09
@roryabraham
Copy link
Contributor

@nbhargava Is this PR still going to be necessary? I read your comment here and it led me to think you might not need to do this anymore?

@nbhargava
Copy link
Author

Yeah probably not. Closing now.

@nbhargava nbhargava closed this Feb 17, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Feb 17, 2021
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.

2 participants