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

Added working version, updated readme, added tests. #11

Closed

Conversation

pejrak
Copy link
Contributor

@pejrak pejrak commented Nov 29, 2016

Resolves #6 , but may be introducing breaking changes compared to the previous package versions, which depended on the index being the second argument of replacer function.

@iansinnott
Copy link
Owner

Awesome! Thanks @pejrak. You're right about the index so this will need to be released as a new major version, which is totally fine.

@iansinnott
Copy link
Owner

@pejrak you feeling good about this one? Maybe we release 1.0 with these changes?

@pejrak
Copy link
Contributor Author

pejrak commented Dec 23, 2016

@iansinnott This is good from functionality stand-point, have been using it in a project since adding. The readme should be good, the only thing I did not fully check is the full example with node server. Are you able to execute that?

@iansinnott
Copy link
Owner

@pejrak ah, thanks for the reminder. I will look into that and fix it as need be

@rpearce
Copy link

rpearce commented Mar 4, 2019

Doesn't look like this has any movement, but I solved the same problem today and have this functionality at https://github.com/rpearce/flexible-string-replace

@iansinnott
Copy link
Owner

Thanks @rpearce. Yeah this one seems to have fallen though the cracks. Going to close it out now as it's two years old

@iansinnott iansinnott closed this Mar 4, 2019
@rpearce
Copy link

rpearce commented Mar 4, 2019

Thanks @iansinnott . @pejrak, if you'd like to be added as a contributor on https://github.com/rpearce/flexible-string-replace, let me know, as your alternative solution & mine were almost identical, and if you still have this problem, perhaps we can iterate there together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants