Skip to content

Rewrite Get-ChocolateyPath cmdlet #3646

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 22, 2025
Merged

Rewrite Get-ChocolateyPath cmdlet #3646

merged 3 commits into from
May 22, 2025

Conversation

vexx32
Copy link
Member

@vexx32 vexx32 commented Mar 3, 2025 •
edited by gep13
Loading

Description Of Changes

  • Replace Get-ChocolateyPath function with C# cmdlet
  • Add tests to verify the expected behaviour of the function

Motivation and Context

See #3477

Testing

  • Run Invoke-Tests.ps1 -Tag GetChocolateyPath

Operating Systems Testing

Windows 11

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation. - see (#1074) Add docs for Get-ChocolateyPath docs#1173
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v3 compatibility checked?

Related Issue

Part of #3477

Docs PR: chocolatey/docs#1173

@vexx32 vexx32 requested review from gep13 and corbob March 3, 2025 21:05
@vexx32 vexx32 force-pushed the get-chocolateypath branch from 557de2c to 69703d8 Compare March 10, 2025 20:27
@vexx32 vexx32 marked this pull request as ready for review March 10, 2025 21:51
@vexx32 vexx32 mentioned this pull request Mar 12, 2025
Open
44 tasks
corbob
corbob requested changes Mar 12, 2025
View reviewed changes
Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

Just a few minor things.

@vexx32 vexx32 force-pushed the get-chocolateypath branch from 69703d8 to a2a717a Compare March 21, 2025 15:24
@vexx32 vexx32 force-pushed the get-chocolateypath branch from a2a717a to 2606d36 Compare April 11, 2025 14:55
corbob
corbob approved these changes Apr 21, 2025
View reviewed changes
Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

This looks good to me. I've run some tests in a local VM, and a full suite of tests in Test Kitchen. None of the failing tests in Test Kitchen are a concern at the moment (see Slack discussion for more details).

@vexx32 vexx32 force-pushed the get-chocolateypath branch from 2606d36 to b2d5af9 Compare May 9, 2025 15:00
gep13
gep13 requested changes May 21, 2025
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

I have a couple of questions/suggestions here.

Let me know what you think, and if you have any follow up questions.

Once these are addressed, and this PR rebased, I think we can go ahead and get this merged.

src/chocolatey.sln Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Why was this file changed?

If this is indeed needed, I think it warrants its own commit to provide some insight into why this was needed.

Copy link
Member

Choose a reason for hiding this comment

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

I think I've seen similar changes when I've been debugging things, so I suspect this was a Visual Studio thing that just automatically added these things.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I probably forgot to pull it out, I can remove this one if we want? VS keeps changing it for me though and it might just be easier to roll that in sooner or later so it's not a constant annoyance.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this one for now; if we wanna pull this one in, it should probably just go on develop at some point anyway rather than this feature branch.

Copy link
Member

Choose a reason for hiding this comment

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

If this is repeatable on your system, it would be great to know what those steps are, as I have not seen these changes being made on my system when I have this sln open. I always tend to defer to what Visual Studio wants to do, since it "knows better" and it means that I stop fighting with it, but I haven't seen this specific change being made, so would be curious to know what is causing it. i.e. could it be a different extension that you have installed, or that I have installed.

@vexx32 vexx32 force-pushed the get-chocolateypath branch from b2d5af9 to e6420f4 Compare May 21, 2025 20:53
vexx32 added 3 commits May 21, 2025 16:58
@vexx32
(chocolatey#3477) Rewrite Get-ChocolateyPath helper command
b393853 
Rewrite of the Get-ChocolateyPath into C# cmdlet, and associated
changes to helper methods.
@vexx32
(maint) Update cmdlet documentation script
45901eb 
Previous changes here made things not work very well with the renaming
of md<=>mdx files, this just ensures that you can actually have the
script open the editor for the MDX files, since those are what exists
after the script runs.
@vexx32
(chocolatey#3477) Add tests for Get-ChocolateyPath
6d79bd3 
Test all the things!
@vexx32 vexx32 force-pushed the get-chocolateypath branch from e6420f4 to 6d79bd3 Compare May 21, 2025 20:58
gep13
gep13 approved these changes May 22, 2025
View reviewed changes
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13 gep13 merged commit 49c941a into chocolatey:feature/cmdlets May 22, 2025
5 checks passed
@gep13
Copy link
Member

gep13 commented May 22, 2025

@vexx32 thanks for getting this added!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corbob corbob corbob approved these changes

@gep13 gep13 gep13 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vexx32 @gep13 @corbob