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

Updates to the PowerShell modules #4716

Merged
merged 19 commits into from
Sep 9, 2024
Merged

Conversation

kilasuit
Copy link
Contributor

@kilasuit kilasuit commented Aug 8, 2024


Microsoft Reviewers: Open in CodeFlow

edit by @denelon - formatting for checkboxes :)

@kilasuit kilasuit requested a review from a team as a code owner August 8, 2024 13:25
@microsoft-github-policy-service microsoft-github-policy-service bot added the Issue-Feature This is a feature request for the Windows Package Manager client. label Aug 8, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@kilasuit
Copy link
Contributor Author

kilasuit commented Aug 8, 2024

FYI I could not build this small part locally at all getting this error no matter what I do to try and build it

error MSB4019: The imported project "C:\Microsoft.Cpp.Default.props" was not found. 
Confirm that the expression in the Import declaration "\Microsoft.Cpp.Default.props" is correct, and that the file exists on disk.

I also was thinking that on push to this repo that a build would be automatically triggered, though looking at other pr's it's manually being kicked off using the /azp run mechanism (which I don't think I would be able to run)

Other than that this is ready to merge if a build completes no issue

@kilasuit
Copy link
Contributor Author

kilasuit commented Aug 8, 2024

/azp run

Copy link

Commenter does not have sufficient privileges for PR 4716 in repo microsoft/winget-cli

@kilasuit
Copy link
Contributor Author

kilasuit commented Aug 8, 2024

was worth a shot 🤷🏻‍♂️

@kilasuit kilasuit changed the title first pass of adding aliases for the client PowerShell module Updates to the PowerShell modules Aug 9, 2024
@kilasuit
Copy link
Contributor Author

kilasuit commented Aug 9, 2024

Just working on adding the additional bits needed for #4680 to get this all added in a single PR

@kilasuit kilasuit force-pushed the aliases branch 2 times, most recently from 2df4144 to 3637914 Compare August 9, 2024 16:39
@kilasuit
Copy link
Contributor Author

@JohnMcPMS thanks for the comments, I'll get back to these during this week (hopefully tonight) as I have some updates on another device that was in progress at the end of the week.

@denelon
Copy link
Contributor

denelon commented Aug 27, 2024

@kilasuit thanks for jumping in here!

I think we'd want to make sure and update the appropriate "Help" files with the changes as well. Assuming this is all "non-breaking" changes, that can be a separate PR. I know we're also adding a new cmdlet for the WinGet Repair command.

@kilasuit
Copy link
Contributor Author

kilasuit commented Aug 27, 2024

So this should be good for merging now & thanks for the ping @denelon - Tests and docs updated to the singular use

Only other thoughts

  • Is the file name changes for the depluralised cmdlet files ok or does this need reverting
  • Is the change to the class name as per 696bab0 ok?
  • Do you want this rebased with cleaner commit history at all or is it fine as is?

@denelon
Copy link
Contributor

denelon commented Aug 27, 2024

If the cmdlets have been renamed to singular, I'd say we should keep them plural unless the help files get included in the PR. Probably best to do the rename in a separate PR so the help files don't break or confuse users. Get-Help and Get-Command should stay in sync.

I'll let one of our engineers make the call on the remaining "other thoughts".

@kilasuit
Copy link
Contributor Author

Thought it best to add all in together so I have updated the helpfiles in this PR - though I can easily remove those commits if desired

@denelon
Copy link
Contributor

denelon commented Aug 28, 2024

OK, we're cutting a build with the current cmdlets and help. As soon as that is out, we can dig back through this PR to get updates for all the changes. I really appreciate the help here!

@JohnMcPMS
Copy link
Member

Looking good to me; we need to fix some infrastructure issues and then we can run the build/test pass.

@denelon
Copy link
Contributor

denelon commented Sep 3, 2024

The following PR was just merged:

Those examples should be included here.

@JohnMcPMS
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kilasuit
Copy link
Contributor Author

kilasuit commented Sep 5, 2024

The following PR was just merged:

* [Updates to PowerShell help docs #4781](https://github.com/microsoft/winget-cli/pull/4781)

Those examples should be included here.

I just rebased from the master branch & resolved the last comment so all good to go :-)

@JohnMcPMS
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback Issue needs attention from issue or PR author label Sep 6, 2024
@JohnMcPMS
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JohnMcPMS
Copy link
Member

Build was successful with my changes. @kilasuit, please sign off on the PR to indicate that you are done with changes and ready to merge.

@kilasuit
Copy link
Contributor Author

kilasuit commented Sep 6, 2024

Yeah all good with me @JohnMcPMS

@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 Sep 6, 2024
@JohnMcPMS JohnMcPMS merged commit 8691cb3 into microsoft:master Sep 9, 2024
9 checks passed
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Attention Issue needs attention from Microsoft label Sep 9, 2024
@kilasuit kilasuit deleted the aliases branch September 10, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature This is a feature request for the Windows Package Manager client.
Projects
None yet
5 participants