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

BREAKING CHANGE: Change all resources to start with 'MSFT_SQLServer' #137

Closed
1 of 2 tasks
johlju opened this issue Sep 24, 2016 · 10 comments
Closed
1 of 2 tasks

BREAKING CHANGE: Change all resources to start with 'MSFT_SQLServer' #137

johlju opened this issue Sep 24, 2016 · 10 comments
Labels
breaking change When used on an issue, the issue has been determined to be a breaking change. enhancement The issue is an enhancement request.

Comments

@johlju
Copy link
Member

johlju commented Sep 24, 2016

In issue #134 @luigilink raised the question that some resources starts with 'MSFT_SQL' while most start with 'MSFT_SQLServer'.
It wasn't addressed in that issue, focus was to change all resources to use the prefix 'MSFT'. But since that is fixed we could continue with this question.

Since changing this would become a breaking change, I set the title accordingly.

All in favor to change the resource names to one naming convention? :) If so, then I also think we should add that naming convention to the contributing section in this modules README.md.

@luigilink
Copy link
Contributor

luigilink commented Sep 24, 2016

👍
Can we also choose a name convention for tests, like:
MSFT_xSQLServerRole.Tests.ps1 => xSQLServerRole.Tests.ps1

Resource need to start with MSFT_ but not the other files if I am correct.

@kwirkykat kwirkykat added enhancement The issue is an enhancement request. breaking change When used on an issue, the issue has been determined to be a breaking change. labels Sep 27, 2016
@johlju
Copy link
Member Author

johlju commented Nov 27, 2016

@luigilink PowerShell team is using the prefix 'MSFT_' for tests over at PSDscResources. So I think we keep it the same.

@johlju
Copy link
Member Author

johlju commented Nov 27, 2016

Naming convention has been added to the specific guidlines

@johlju
Copy link
Member Author

johlju commented Apr 19, 2017

There is two suggestions for renaming the resource xWaitForAvailabilityGroup. And of course there is always the option to keep the name as is.

@randomnote1 suggested 'xWaitForAlwaysOnAvailabilityGroup'. Which would better explain that it is Always On Availability Groups it handles, and not any other Availability Groups.

@johlju suggested xSQLServerAlwaysOnWaitForAvailabilityGroup? It is long, but it is similar to the other AlwaysOn resource, and it would be easier to find the resource with Get-DscResource xSQLServer*.

@johlju
Copy link
Member Author

johlju commented Apr 19, 2017

To all: Please comment what you think of renaming the resource xWaitForAvailabilityGroup.

@randomnote1
Copy link
Contributor

I vote for 'xSQLServerAlwaysOnWaitForAvailabilityGroup'. This is more consistent with the rest of the resources.

@randomnote1
Copy link
Contributor

In light of the resource name length issue presented in in issue #716 , I think we should resume discussion and expand upon this issue.

A common theme I've been reading in the issues is that the names of the resources in the xSQLServer module are too long. Even if they worked with Azure Automation, they are considered to be too long. With that said, some of the names are extremely long due to the naming conventions that have been adopted (eg. xSQLServerAlwaysOnAvailabilityGroupDatabaseMembership).

The basic format of the naming convention appears to be:
<Module Identifier><Action><Feature><Feature Property>

If we could publish guidance on what our naming convention is for each item, I think it could result in shorter name lengths. This published guidance would be part of the contributing guidelines. For example:

  • Module Identifier
    • SQL - I think xSQLServer for the module identifier is redundant since the module is named xSQLServer. This is also seven characters shorter. A good reference for this is the SharePointDSC module. They use "SP" as the prefix for all their resources.
  • Action (can be omitted)
    • WaitFor
  • Feature (List is incomplete)
    • AvailabilityGroup - I don't think we need the "AlwaysOn" qualifier for this.
    • Database
  • Feature Property (can be omitted)
    • DatabaseMembership
    • RecoveryModel

Some examples using this proposed method:

  • SQLAlwaysOnDatabaseMembership
  • SQLDatabaseRecoveryModel
  • SQLSetup
  • SQLWaitForAvailabilityGroup

I'm looking forward to hearing your thoughts on this.

@johlju
Copy link
Member Author

johlju commented Sep 28, 2017

@randomnote1 Thanks for bringing the issue of naming convention up, and coming with a good suggestion. Could you please move this to a separate issue for naming convention?
Then this issue will be obsolete it we move ahead with changing the naming convention.

@randomnote1
Copy link
Contributor

Created issue #851 to track this.

@johlju
Copy link
Member Author

johlju commented Dec 22, 2017

This has been resolved in issue #851.

@johlju johlju closed this as completed Dec 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change When used on an issue, the issue has been determined to be a breaking change. enhancement The issue is an enhancement request.
Projects
None yet
Development

No branches or pull requests

4 participants