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

Add # support for LabelValidator as discussed in #2006 #2059

Closed
wants to merge 2 commits into from

Conversation

wsxiaoys
Copy link

@wsxiaoys wsxiaoys commented Nov 9, 2016

As suggested by @damienmg in #2006 send this out to run it with internal test.

@bazel-io
Copy link
Member

bazel-io commented Nov 9, 2016

Can one of the admins verify this patch?

@googlebot
Copy link

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).

📝 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. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@wsxiaoys
Copy link
Author

wsxiaoys commented Nov 9, 2016

I'm a Googler so I don't need to sign CLA.

@aehlig
Copy link
Contributor

aehlig commented Nov 9, 2016

over to @damienmg who apparently made the suggestion.

Please note that, as the account is not registered with the google oranisation,
we have no means to verify the claim "I'm a Googler".

mzh0 added 2 commits November 9, 2016 10:58
Add # to LabelValidator allowed list for query target.
Add test case for #
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Nov 9, 2016
@wsxiaoys
Copy link
Author

wsxiaoys commented Nov 9, 2016

I updated my information and commits and seems cla check automatically passed.

@damienmg
Copy link
Contributor

Oops sorry, I forgot to import it yesterday.

Yes using your @google.com email for the author in the commit fix it (which you should do if you are employed by google).

@wsxiaoys
Copy link
Author

Great, thank you. Does it break anything?

@wsxiaoys
Copy link
Author

ping?

@damienmg
Copy link
Contributor

Sorry I sent the change to @lberki for review but he seemed to have missed
it. Our tests have passed so far

On Thu, Nov 17, 2016 at 5:50 PM Meng Zhang notifications@github.com wrote:

ping?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2059 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADjHfzxdVMhUHIrQIizFTEq67Wv74sZLks5q_IXFgaJpZM4KtFVC
.

@lberki
Copy link
Contributor

lberki commented Nov 18, 2016

Let's see what happens. We generally prefer patches through Gerrit, but it's a tiny simple one so I can just patch it in manually and run it through our internal test battery.

@bazel-io bazel-io closed this in 84a3ed9 Nov 18, 2016
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this pull request Sep 5, 2022
    As suggested by @damienmg in #2006 send this out to run it with internal test.

    Closes #2059 .

    Progress towards #374.

    --
    Reviewed-on: bazelbuild/bazel#2059
    MOS_MIGRATED_REVID=139562084
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants