search

0xProject / 0x-launch-kit-frontend

Apache License 2.0
114 stars 208 forks source link

(Feature) Moved tests to src/tests folder #299

Closed Agupane closed 5 years ago

Agupane commented 5 years ago

Closes #291

unjapones commented 5 years ago

Question validating change: Should the test folder be in the root of the project (not in the src folder) ?

I don't have a strong opinion here, but it looks like other 0x projects have the test folder in the same level as src as you mentioned (Instant, contract-wrappers, web3-wrapper) .

@Agupane do you see any downside of doing it like the other 0x projects?

fvictorio commented 5 years ago

Oh, I totally missed that. Yeah, it should be at the root level.

Agupane commented 5 years ago

In order to move the tests folder to the root folder we need to make an npm eject an re-configure webpack. Moving the tests to root folder is not a react-convention as show here. That's why they don't let them run outside src. They opened an issue here to make that.

If you guys think it's ok, I'll continue with the eject and configure webpack to run tests from root. @unjapones @mariano-aguero @fvictorio

unjapones commented 5 years ago

In order to move the tests folder to the root folder we need to make an npm eject an re-configure webpack. Moving the tests to root folder is not a react-convention as show here. That's why they don't let them run outside src. They opened an issue here to make that.

If you guys think it's ok, I'll continue with the eject and configure webpack to run tests from root. @unjapones @mariano-aguero @fvictorio

Yup, looks like create-react-app enforces to have test inside /src (in my mind, it makes sense).

I prefer to not-eject unless we really need it, so I would keep things like they already are in this PR and approve it. If the rest of you guys think the same, feel free to approve & merge. Then we can answer the comment that originated this change.