Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

[configure-splash-screen]<chore>: Update test file names to follow naming convention #2749

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

chernandez7
Copy link
Contributor

@chernandez7 chernandez7 commented Oct 2, 2020

TODO


  • Confirm XML naming convention
  • Make sure tests are passing on CircleCI

Description


As mentioned in issue #2591 and PR #2297, we want to update tests from .test to -test.

Only question I have is relating to the .xml files. Should those also follow the convention mentioned above or stay as xml.test?

Also, it's my first PR towards Expo so if I missed any conventions to follow feel free to let me know.

Test Plan


I don't believe much testing would be needed since it's just renaming files.

I ran the tests on the package but some were failing and it seems like they're related to my environment or even OS. But would like some confirmation on if that's true or not.

Ex. 1:
  -     "image": "/assets/background.png",
  +     "image": "E:\\assets\\background.png",

Ex. 2:
    -   "image": "/assets/background.png",
    +   "image": "E:\\assets\\background.png",

Ex. 3:
 EEXIST: file already exists, mkdir 'E:\app\android\app\src\main\res\drawable'

      at createError (../../node_modules/memfs/lib/volume.js:121:17)
      at Volume.mkdirBase (../../node_modules/memfs/lib/volume.js:1527:19)
      at Immediate.<anonymous> (../../node_modules/memfs/lib/volume.js:681:39)

@EvanBacon
Copy link
Contributor

You can ensure it works on your machine by running the tests on master.

Copy link
Contributor

@bbarthec bbarthec left a comment

Choose a reason for hiding this comment

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

This LGTM 👍
Tests are fired and are passing ✅
I like the .xml naming as you're proposing 😉

@bbarthec bbarthec merged commit 2e69621 into expo:master Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants