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

Clarify settings behavior #5062

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Gijsreyn
Copy link
Contributor

@Gijsreyn Gijsreyn commented Dec 13, 2024


When looking through the discussions, I noticed the following question asked by Tristan. After trying out some options, I thought the ConfigureBehavior would do the trick. Unfortunately, after trying it out, I took the conclusion from the description that only winget install is affected by these settings. This pull request clarifies it a bit more.

Microsoft Reviewers: Open in CodeFlow

@Gijsreyn Gijsreyn requested a review from a team as a code owner December 13, 2024 10:18
Copy link
Contributor

@Trenly Trenly left a comment

Choose a reason for hiding this comment

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

They affect more than just install. If I'm not mistaken, they also affect upgrade and import.

In fact, I would think it's nore likely a bug in DSC if they aren’t being considered when used with the WinGetPackage resource; especially becausethefe doesn’t seem to be Scope or Locale or InstallerType properties on the DSC Resource (@denelon - should these be added? Should I open an issue for that?)

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Dec 13, 2024
@Gijsreyn
Copy link
Contributor Author

Thanks for the reply Kaleb. If that's the case, then I would even think of renaming the setting name, as it states installBehavior, but that might be a preference and opinion.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention Issue needs attention from Microsoft and removed Needs-Author-Feedback Issue needs attention from issue or PR author labels Dec 13, 2024
@Trenly
Copy link
Contributor

Trenly commented Dec 13, 2024

Thanks for the reply Kaleb. If that's the case, then I would even think of renaming the setting name, as it states installBehavior, but that might be a preference and opinion.

Unfortunately I think that renaming the setting would be considered a breaking change (unless both the old name and the new name would work)

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Attention Issue needs attention from Microsoft label Dec 13, 2024
@Gijsreyn
Copy link
Contributor Author

I agree; maybe only introducing upgradeBehavior would suffice, as uninstallBehavior is already there.

@Trenly
Copy link
Contributor

Trenly commented Dec 13, 2024

I agree; maybe only introducing upgradeBehavior would suffice, as uninstallBehavior is already there.

Then you get into weird semantics because an upgrade is just a special case of install

@denelon
Copy link
Contributor

denelon commented Dec 13, 2024

In general, a WinGet Configuration shouldn't be affected by a user's Package Management settings. That's probably something we'll need to clarify.

@Trenly
Copy link
Contributor

Trenly commented Dec 13, 2024

In general, a WinGet Configuration shouldn't be affected by a user's Package Management settings. That's probably something we'll need to clarify.

In that case, does the DSC Resource need to be updated to support Scope, Locale, and installer type? It seems like a gap between the DSC Resource and the WinGet Client if there isn’t a way to specificy which is the preferred scope, or preferred installer

@denelon
Copy link
Contributor

denelon commented Dec 13, 2024

Yes, we have an issue open for that work as well.

@denelon
Copy link
Contributor

denelon commented Dec 13, 2024

@Trenly
Copy link
Contributor

Trenly commented Dec 13, 2024

Yes, we have an issue open for that work as well.

Ah, I knew I was missing something!

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.

3 participants