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.

Sign up for GitHub

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

Jump to bottom

Add support for string.IndexOf(string value, int startIndex) #1645

Merged
merged 3 commits into from
May 15, 2022
Merged

Add support for string.IndexOf(string value, int startIndex) #1645

merged 3 commits into from
May 15, 2022

Conversation

LeaFrock
Copy link
Contributor

@LeaFrock LeaFrock commented May 11, 2022 •
edited by lauxjpn
Loading

Fixes #1625

@LeaFrock LeaFrock mentioned this pull request May 11, 2022
Closed
@mguinness
Copy link
Collaborator

Can you add some functional tests, see NorthwindFunctionsQueryMySqlTest class for examples.

@LeaFrock
Copy link
Contributor Author

Can you add some functional tests, see NorthwindFunctionsQueryMySqlTest class for examples.

Of course. I'll handle it tomorrow, thanks.

@LeaFrock
Copy link
Contributor Author

Functional tests added. @mguinness
There seems to be a mistake for these two test methods, StringIndexOf_invariant & StringIndexOf_current.

1652336118(1)

If you're sure that it's a typo, I'll add a separate commit for fix.

@mguinness mguinness requested a review from lauxjpn May 12, 2022 23:12
lauxjpn
lauxjpn requested changes May 13, 2022
View reviewed changes
Copy link
Collaborator

@lauxjpn lauxjpn left a comment

Choose a reason for hiding this comment

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

Looks good!

There seems to be a mistake for these two test methods, StringIndexOf_invariant & StringIndexOf_current.

Yes, please go ahead and just add another commit fixing those.

I'll fix the CI error regarding the Windows environment job, which is unrelated to your changes.

@lauxjpn lauxjpn added this to the 7.0.0 milestone May 13, 2022
@LeaFrock LeaFrock requested a review from lauxjpn May 14, 2022 05:03
@lauxjpn
Copy link
Collaborator

lauxjpn commented May 14, 2022

The Windows CI pipeline is fixed now.

@LeaFrock Please rebase your commits on top of the latest master commit and (force) push again.

The checks should succeed then and we will be able to merge your PR.

@LeaFrock
Copy link
Contributor Author

@lauxjpn Done.

@lauxjpn lauxjpn merged commit 8954910 into PomeloFoundation:master May 15, 2022
@lauxjpn
Copy link
Collaborator

lauxjpn commented May 15, 2022

@LeaFrock Thank you for your contribution!

@LeaFrock LeaFrock mentioned this pull request May 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lauxjpn lauxjpn lauxjpn approved these changes

Assignees

@mguinness mguinness

Labels
type-enhancement
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

Support for int IndexOf(string value, int startIndex)
3 participants
@LeaFrock @mguinness @lauxjpn