guava
guava copied to clipboard
google
base.Ascii: Add methods indexOfIgnoreCase, containsIgnoreCase, startsWithIgnoreCase and endsWithIgnoreCase
Implements the changes proposed in issue #3011
I was not sure in which version this is going to be pulled in - so I left // TODO comments at the @since tags. Feel free to tell me which number to change it to 😃
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
:memo: Please visit https://cla.developers.google.com/ to sign.
Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.
- If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.
- If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot. The email used to register you as an authorized contributor must be the email used for the Git commit.
- In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.
You're welcome 😃 thanks a lot, too, for the very thorough and good code reviews of you all, they were very constructive and helpful. I now squashed the commits. Maybe one of you Googlers can run the benchmark (I hope I did it right)? I don't think there is any possibility to run them myself. Would be interesting to see the performance differences. Besides that, from my side it's good to merge.
Hey there, just wanted to ask if there's any news concerning this pull request?
@martinfrancois I'm not on the Guava team myself, so I can't truly speak on their behalf, but as someone who's contributed to the project for a couple of years now, I think it's worth letting you know that they can be very slow addressing new issues and PRs, which I think happens because they're busy as a whole dealing with Google-internal priorities.
So if they take forever to respond (which seems likely at this stage, sadly), it absolutely won't be because you did something wrong; they're just apparently very, very busy with their day-to-day jobs, and someone from the team will eventually respond to this PR.
If you don't get a response as quickly as you'd like, writing a reminder message like the one you wrote here every now and then should encourage a more timely response, even if they're a bit too busy to review the PR properly.
I hope this helps! :)
There are various reasons that we haven't done a good job about replying to issues. Busyness is part of it, but we should do better. It looks like our current triage process misses pull requests. I'll try to get that fixed.
@jbduncan thanks for the very kind words and the tips 😃 I appreciate that a lot, it's good to know that it's not an ununsual case.
@cpovirk also thanks for taking it serious and trying to get pull requests integrated into your triage process 👍
Thanks for accepting my pull request! I wanted to ask when the changes of it are going to get synced out? I haven't seen it on the master yet.
I'm sorry that we are so slow on this. Fundamentally we appreciate the request, but we have a bit of work to do on our end that we haven't been able to prioritize yet.
FYI, I do see a pretty significant amount of Google code writing stuff like if (s.toLowerCase().startsWith(...)) so I do think the case for adding these methods to Guava may be a pretty good one.
@kevinb9n thanks for the update on this! It has now been almost half a year, are there any internal processes it still needs to go through or did the team just not get around to merging it into the codebase yet? Thanks!
I want to quickly note here that when an issue or PR is marked "triaged" and assigned to someone, that doesn't necessarily mean it's accepted. Triaged just means that someone has looked at it and thinks it's well-formed and worth thinking about more, and assigned it to someone who they think should best be able to consider it. That said, Kevin has noted that this seems like something that there's a good chance we'll want to do.
Another note: API additions do generally need to go through an API review process we have internally, which tends to involve writing up a doc exploring pros and cons of an API, basic statistics for how often existing code in our codebase would want to use the API(s), options for how to address the problem, etc., followed by scheduling it to be discussed with a group of reviewers. That's obviously a bit of work, so even when you've written the code, it can be hard to fit it in with everything else we're doing. That's not to say we can't do better--we can and should do better, and are working on improving our processes around bugs/PRs--but just to give some idea of why things might take so long sometimes.
Thanks a lot @cgdecker! I tried to find a description of your process but was unable to. Your description made it very clear to me. Now I understand what goes into it and it explains the long process, but I do appreciate your attention to detail and sense of quality, it gives me an even greater confidence in the high standards of the guava code base.
Hi everyone, I don't want to sound impatient, but it's been almost a year now. Are there any news on this one? Thanks, François
François Martin has personally and elaborately explained to me this PR and it's history. Actually he was so kind to copy over the implementation into a project we are working on together. I vote for merging this PR.
@kevinb9n or @cgdecker could I please get an update on the current status of your internal review process concerning this PR?