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

Change SQL Server codenames to version names #1334

Closed
wants to merge 5 commits into from

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Oct 12, 2021

This changes the sql server codenames found in the code to the released version numbers as found at https://en.wikipedia.org/wiki/List_of_Microsoft_codenames#SQL_Server_family

I thought this would be a fairly small set of changes when i started but it has ended up touching a lot of files. There are no functional changes.

@cheenamalhotra
Copy link
Member

It seems there are syntax errors:
src\Microsoft.Data.SqlClient\netcore\src\Microsoft\Data\SqlClient\TdsParser.cs(3603,79): Error CS1003: Syntax error, ':' expected

Please fix and check-in..

@Wraith2
Copy link
Contributor Author

Wraith2 commented Oct 12, 2021

Fixed.

@Wraith2
Copy link
Contributor Author

Wraith2 commented Oct 14, 2021

CI doesn't like this but as far as I can see none of the errors are due to my changes.

@JRahnama
Copy link
Contributor

Can you address the conflict here please?

@Wraith2
Copy link
Contributor Author

Wraith2 commented Dec 20, 2021

I changed SqlCommandBuilder to have the DesignerCategory("") which prevents the SubType coming back. I also reverted the auto expansion of the resx localized imports though the designer keeps wanting to put them back which is unhelpful.

@@ -39,7 +39,7 @@ internal class Strings {
internal static global::System.Resources.ResourceManager ResourceManager {
get {
if (object.ReferenceEquals(resourceMan, null)) {
global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Data.SqlClient.Resources.Strings", typeof(Strings).Assembly);
global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("SqlClient.Resources.Strings", typeof(Strings).Assembly);
Copy link
Contributor

Choose a reason for hiding this comment

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

Failures should came from these change!

Suggested change
global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("SqlClient.Resources.Strings", typeof(Strings).Assembly);
global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("Microsoft.Data.SqlClient.Resources.Strings", typeof(Strings).Assembly);

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. The resource naming needs to be looked at sometime in a far future when we have time for such small issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems it hasn't been fixed! 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. And I don't understand why it's broken or how i broke it in this PR. I may have to close and start again on a clean branch.

@Wraith2
Copy link
Contributor Author

Wraith2 commented Dec 21, 2021

I don't understand the source of problems in this PR so I'm going to close and rework clearly on a new branch. Was there any comment or issue on the renames themselves?

@Wraith2 Wraith2 closed this Dec 21, 2021
@DavoudEshtehari
Copy link
Contributor

DavoudEshtehari commented Dec 21, 2021

As my understanding if you follow my code suggestion and revert the last commit it'll be fixed. Please, give it a try before making a new PR.

@Wraith2
Copy link
Contributor Author

Wraith2 commented Dec 22, 2021

I tried, it didn't. That combined with the earlier changes that were resolved by rebasing concerned me enough that I'd rather do it again cleanly than risk an unclean branch causing later issues.

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.

None yet

5 participants