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

FIX | Add Component Model annotations to SqlConnectionStringBuilder class in netcore's src folder #1152

Merged
merged 7 commits into from Aug 20, 2021
Merged

FIX | Add Component Model annotations to SqlConnectionStringBuilder class in netcore's src folder #1152

merged 7 commits into from Aug 20, 2021

Conversation

JRahnama
Copy link
Member

@JRahnama JRahnama commented Jul 6, 2021

fixes #1142

@JRahnama JRahnama changed the title FIX | Add Component Model annotations to netcore src folder FIX | Add Component Model annotations to SqlConnectionStringBuilder class in netcore's src folder Jul 6, 2021
@JRahnama JRahnama added this to the 4.0.0-preview1 milestone Jul 6, 2021
@JRahnama JRahnama self-assigned this Jul 6, 2021
@JRahnama JRahnama force-pushed the add-componentmodel-to-netcoresrc branch from 7cb962c to 4c3847c Compare July 8, 2021 22:20
@JRahnama JRahnama force-pushed the add-componentmodel-to-netcoresrc branch from 54e58ef to e7e1040 Compare July 14, 2021 18:29
@JRahnama
Copy link
Member Author

Just an update here, I manually added the partial class of StringsHelper with all the needed constant strings inside it. In netfx there is a script named GenerateResourceStringsSource.ps1 and runs BeforeTargets="CoreCompile" which is called by GenerateResourceStringsSource.targets and it adds the partial class of StringsHelper inside each output directory in obj folder as Strings.ResourceNames.cs. In netcore, running the powershell script comes up with some issues on Linux machines. One of the options that could be investigated in future, when we have localization for netcore available, is MetadataTypeAttribute, which could be added to SqlConnectionStringBuilder.Windows and make sure this is only called on Windows OSs. The only problem with this class is netcoreapp2.1 which does not support it, however netcoreapp2.1 will come to EOL in August 2021 and that wont be problem anymore. We will look for other options when get to it.

cheenamalhotra
cheenamalhotra approved these changes Jul 16, 2021
View reviewed changes
Copy link
Member

@cheenamalhotra cheenamalhotra left a comment

Choose a reason for hiding this comment

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

Just a tiny change, rest looks good to me 👍

src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj Outdated Show resolved Hide resolved
@cheenamalhotra cheenamalhotra added this to In progress in SqlClient v4.0 via automation Jul 16, 2021
@cheenamalhotra cheenamalhotra moved this from In progress to Review in progress in SqlClient v4.0 Jul 16, 2021
JRahnama and others added 4 commits August 17, 2021 14:20
@JRahnama
adding attributes to SqlCommand and SqlConnection in netcore
1a3545b
@JRahnama
commit
abcebfd
@JRahnama
commit
a772e0d
Merge branch 'main' of https://github.com/JRahnama/SqlClient into add…
58c51f2 
…-componentmodel-to-netcoresrc

# Conflicts:
#	src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.NetCoreApp.cs
#	src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionStringBuilder.cs
@DavoudEshtehari
Copy link
Member

Here are my observation of some types:
Properties descriptions seem good. 👏
But categories need some modifications.

Net Core Net fx
image image
image image
image image
image image
image image

@JRahnama
Copy link
Member Author

Here are my observation of some types:
Properties descriptions seem good. 👏
But categories need some modifications.

I have added as many as possible. In some cases the base classes such as DataAdapter are adding those meta tags in netfx and in netcore those base classes do not have them. In order to do so we have to add them manually and it requires adding the strings to string.resx, I changed as many as I could. At some other places such SqlDataAdapter AcceptChangesDuringFill is in the base class and we have no control over them.

DavoudEshtehari
DavoudEshtehari approved these changes Aug 20, 2021
View reviewed changes
Copy link
Member

@DavoudEshtehari DavoudEshtehari left a comment

Choose a reason for hiding this comment

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

I believe these changes are enough for this PR.

SqlClient v4.0 automation moved this from Review in progress to Reviewer approved Aug 20, 2021
@cheenamalhotra cheenamalhotra merged commit d1ff708 into dotnet:main Aug 20, 2021
SqlClient v4.0 automation moved this from Reviewer approved to Done Aug 20, 2021
@cheenamalhotra cheenamalhotra mentioned this pull request Aug 30, 2021
Merged
@JRahnama JRahnama mentioned this pull request Sep 22, 2021
Closed
@Wraith2 Wraith2 mentioned this pull request Sep 24, 2021
Merged
@JRahnama JRahnama deleted the add-componentmodel-to-netcoresrc branch September 29, 2022 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cheenamalhotra cheenamalhotra cheenamalhotra approved these changes

@DavoudEshtehari DavoudEshtehari DavoudEshtehari approved these changes

@johnnypham johnnypham Awaiting requested review from johnnypham

Assignees

@JRahnama JRahnama

Labels
netcore
Projects
No open projects
SqlClient v4.0
Done
Milestone
4.0.0-preview1
Development

Successfully merging this pull request may close these issues.

PropertyGrid and SQLConnectionStringBuilder
3 participants
@JRahnama @DavoudEshtehari @cheenamalhotra