-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Folder restructure for Search API SDKs [ EntitySearch,WebSearch,ImageSearch,VideoSearch,SpellCheck,CustomSearch ] #3930
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
Conversation
shahabhijeet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, but looking at the directory names, make sure you are not getting into MAX_PATH issues during build.
Also I have seen you have not pulled upstream changes to these project files.
Almost all project files need the following header import and footer imports as you see in this file
https://github.com/sudhansu88/azure-sdk-for-net/blob/psSdkJson6/src/SDKs/Compute/Management.Compute/Microsoft.Azure.Management.Compute.csproj
Also make sure you don't explicitly add any directories or files to the sdk project.
I see couple of places you have added Folder Include="Properties" (an additional ItemGroup). Please remove any ItemGroup that you don't need.
shahabhijeet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes required. Once done this PR is good to be merged.
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <!-- Please do not move/edit code below this line --> | ||
| <Import Project="$([MSBuild]::GetPathOfFileAbove('AzSdk.reference.props'))" /> | ||
| <!-- Please do not move/edit code below this line --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sudhansu88 this should be
Please do not move/edit code above this line and it says "below"
Can you make the change in all the csproj files.
|
Updated Comment as per suggestion |
|
@sudhansu88 Please include the specific name of the service in your PR titles. There is more than one "Search API/SDK" in this repo. Thanks! |
|
@brjohnstmsft : Updated all SDK names. We ares omewhat blocked on this. Could you please help merge ? |
Description
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Testing Guidelines
SDK Generation Guidelines
*.csprojandAssemblyInfo.csfiles have been updated with the new version of the SDK.