Skip to content

Conversation

@ldesplat
Copy link
Contributor

@ldesplat ldesplat commented Mar 6, 2015

  1. Since we specify transform, our tests themselves could be transformed.
  2. Document a pitfall, which is to not transform too many files so the user has to check for that in transform method.

README.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be ===

@geek geek added the feature New functionality or improvement label Mar 6, 2015
@geek geek added this to the 5.5.0 milestone Mar 6, 2015
@geek geek self-assigned this Mar 6, 2015
lib/cli.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This will create a wrong regex ( .(js|)$ ) if the transform module exports an empty array. Also, the dot should be escaped, below and above too.

@ldesplat
Copy link
Contributor Author

ldesplat commented Mar 8, 2015

Thank you so much @gergoerdosi for spotting those issues.

I have fixed the code review issues and file extensions can now include some crazy stuff :). Tested a whole bunch of special characters on Mac, quite interesting!

@geek
Copy link
Member

geek commented Mar 9, 2015

@ldesplat do we need any new tests for any of the edge cases you were considering with this change?

@ldesplat
Copy link
Contributor Author

don't merge

@ldesplat
Copy link
Contributor Author

Ok, I am sorry. All should be well now.

Thanks for telling me to add tests.
There was a bug where applying transforms with the no code coverage path was different than the coverage path. They are now using the exact same code and it is consistent behavior. This is covered in the 2 new tests.

I had to resort to way too many PRs to add this feature in. Next time I'll hold on a bit longer no matter my enthusiasm! Thanks for all your support.

@AdriVanHoudt
Copy link
Contributor

@ldesplat thanks for all the effort to add this feature!

geek added a commit that referenced this pull request Mar 10, 2015
Allow test files to have different extensions with transform
@geek geek merged commit 42e983e into hapijs:master Mar 10, 2015
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants