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 reflection-based Search index definition #2536

Merged
merged 1 commit into from
Nov 14, 2016

Conversation

idg10
Copy link
Contributor

@idg10 idg10 commented Nov 10, 2016

This implements #2427

For the unsquashed history see https://github.com/idg10/azure-sdk-for-net/tree/reflection-schema which includes discussion with @brjohnstmsft as he reviewed my work on this feature.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azurecla
Copy link

Hi @idg10, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.azure.com.

TTYL, AZPRBOT;

@azurecla
Copy link

@idg10, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, AZPRBOT;

}
else if ((indexAnalyzerAttribute = attribute as IndexAnalyzerAttribute) != null)
{
field.IndexAnalyzer = AnalyzerName.Create(((IndexAnalyzerAttribute) attribute).Name);
Copy link
Member

Choose a reason for hiding this comment

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

I missed this earlier. It looks like the cast to IndexAnalyzerAttribute is unnecessary if you use the indexAnalyzerAttribute variable instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f46c44c (resquashed)

/// Standard ASCII Folding Lucene analyzer.
/// <see href="https://msdn.microsoft.com/library/azure/mt605304.aspx#Analyzers" />
/// </summary>
public static readonly AnalyzerName StandardAsciiFoldingLucene =
Copy link
Member

Choose a reason for hiding this comment

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

This should be a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f46c44c (resquashed)

@idg10
Copy link
Contributor Author

idg10 commented Nov 11, 2016

I've fixed both of the problems you found (the unnecessary repeated cast, and my bizarre error in AnalyzerName.AsString.StandardAsciiFoldingLucene). I presumed you still want this to be a single commit in the event that you accept the PR, so I've squashed it again and force pushed to my PR branch.

@markcowl
Copy link
Member

@azuresdkci add to whitelist

@markcowl
Copy link
Member

markcowl commented Nov 12, 2016

LGTM once the build passes

@brjohnstmsft
Copy link
Member

@markcowl The build passed; Please merge

@cormacpayne cormacpayne merged commit fac98e5 into Azure:AutoRest Nov 14, 2016
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.

6 participants