Skip to content
This repository was archived by the owner on Jul 4, 2025. It is now read-only.

Conversation

@jamesplease
Copy link
Collaborator

@pete-the-pete did some great work adding string tests over in #66. In this PR, I'm going to do some final tidying up of the PR, then squash it down for merge.

@jamesplease
Copy link
Collaborator Author

Mmk @ashleygwilliams / @kadamwhite / @pete-the-pete – I tidied up this PR, and I'd love for y'all to review it if you had time.

The last thing I want to do is squash the commits, but I figure I can do that right before merging.

@kadamwhite
Copy link
Contributor

The tests look OK, but it needs to be updated to account for #112: the strings module uses the old require declaration, rather than the exports.namespace = {} strategy.

@ashleygwilliams
Copy link
Collaborator

this looks good to me @jmeas - if you squash i'll merge

@jamesplease
Copy link
Collaborator Author

this looks good to me @jmeas - if you squash i'll merge

...I think I somehow managed to miss this comment altogether. I'll update thiz

@jamesplease
Copy link
Collaborator Author

Updated @ashleygwilliams

@ashleygwilliams
Copy link
Collaborator

awesome @jmeas! merging

ashleygwilliams added a commit that referenced this pull request Aug 1, 2015
@ashleygwilliams ashleygwilliams merged commit 0303e8c into master Aug 1, 2015
@ashleygwilliams ashleygwilliams deleted the string-tests branch August 1, 2015 17:02
@ashleygwilliams
Copy link
Collaborator

🎀

@jamesplease
Copy link
Collaborator Author

Thanks, @ashleygwilliams! now I just need to get together some solutions :)

kumarkalra added a commit to kumarkalra/js-assessment that referenced this pull request Mar 14, 2016
add String tests

Signed-off-by: Kumar Kalra <[email protected]>
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.

6 participants