Skip to content

Changing contract names for EM and Power notifications.#2278

Closed
dhoehna wants to merge 2 commits intomainfrom
user/dahoehna/PowerNotificationContractUpdate
Closed

Changing contract names for EM and Power notifications.#2278
dhoehna wants to merge 2 commits intomainfrom
user/dahoehna/PowerNotificationContractUpdate

Conversation

@dhoehna
Copy link
Copy Markdown
Contributor

@dhoehna dhoehna commented Mar 16, 2022

Contracts were added while the spec was being fleshed out. Changing to a spec compliant name.

Adding contracts to Environment Manager.

@dhoehna
Copy link
Copy Markdown
Contributor Author

dhoehna commented Mar 16, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@dhoehna dhoehna changed the title Changing contract name to be spec compliant. Changing PowerNotification contract name to be spec compliant. Mar 16, 2022
namespace Microsoft.Windows.System
{
[contractversion(1)]
apicontract WindowsSystemContract{};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is rather broad. I expected EnvironmentManagerContract

There was a (tactical/concrete) question about WinAppSDK contract names and API Review. Let me check on that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

#2110 (comment)

@jonwis said WindowsSystemContract.

Should the naming convention be "Something that describes the type", or "Namespace"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I thought the naming convention was by "feature" or "feature set". Namespace is redundant, because the contract is already in a namespace, so this contract is currently named:
Microsoft.Windows.System.WindowsSystemContract

That's no good at all. I'm putting in another vote for EnvironmentManagerContract.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The only item in the spec that talks about contract naming is the rule "Contract names need to end with the word Contract."

I like EnvironmentManagerContract as well. Looks like there needs to be a talk about this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Contracts are units of versioning. I'm looking for a squishy middle here - UniversalApiContract is much too large. FooTypeContract is much too small. Other discussions suggest that a contract of 1-2 types is not interesting.

I think I'd go with WindowsSystemContract for now. Ideally we get one contract per "block of functionality" ... I know that's not a satisfying answer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dhoehna Is this still applicable?

@dhoehna dhoehna changed the title Changing PowerNotification contract name to be spec compliant. Changing contract names for EM and Power notifications. Mar 17, 2022
@bpulliam
Copy link
Copy Markdown
Contributor

bpulliam commented Apr 3, 2023

Closing PRs that appear to have been abandoned.

@bpulliam bpulliam closed this Apr 3, 2023
@bpulliam bpulliam deleted the user/dahoehna/PowerNotificationContractUpdate branch November 7, 2023 13:23
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.

5 participants