Skip to content

Fixed MultiDelimiterStringSearchInterpolator escape String code #4

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

Closed
wants to merge 3 commits into from

Conversation

belingueres
Copy link
Contributor

No description provided.

@khmarbaise
Copy link
Member

If I correctly understand this fixed #3 ? If yes than it would be great to mention something like:

Fixed #3
 o Explanations

Using the Fixed #3 will automatically close the related issue if i merge this into the code base...

ReflectionValueExtractor version with capacity to parse expressions with
arrays, lists and maps.
@khmarbaise
Copy link
Member

khmarbaise commented Jul 9, 2016

Is this related to issue #3 ? It does not look like. So it would be great to make a separate issue for that and a separate pull request otherwise it's hard follow changes and the history in git...and also for end users which issue fixes what in detail...and of course for me to decide to integrate this change?

@belingueres
Copy link
Contributor Author

belingueres commented Jul 9, 2016

Hi!
Thanks for reviewing the patch.
My only intention was only that all tests to pass. Looking at issue #3, it seems somebody commited the only one test failing and this patch fixes it (however I'm not sure if this is the correct behavior or the library). I just added the code to make the test pass and added one test with the double escape character in different order.

Note: I'm new with github contributions so any help will be welcomed.

@belingueres
Copy link
Contributor Author

Hi!
Added a new commit to solve issue #5. (Don't know how to separate it from the first pull request yet.)

@belingueres
Copy link
Contributor Author

Hi @khmarbaise:
Now that I'm a little more experienced with git (oopss), would you agree if I close this pull request and start all over again? (fixing each bug inside their own branch?)
Best Regards,
Gabriel

@khmarbaise
Copy link
Member

Great...If you like sure..and yes fixing each issue in it's own branch makes sense...

…t. The reason is that I've made a mistake and created a unique PR to solve several issues, which is a bad practice.
@belingueres
Copy link
Contributor Author

Replace by PR #7 and #8. This PR will be closed now.

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.

2 participants