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

Clean up psnotify chocoinstall #718

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Clean up psnotify chocoinstall #718

merged 1 commit into from
Oct 31, 2023

Conversation

emtuls
Copy link
Member

@emtuls emtuls commented Oct 21, 2023

No description provided.

@emtuls emtuls requested review from vm-packages and Ana06 and removed request for vm-packages October 21, 2023 03:26
Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

@emtuls please do not merge PRs without a review (specially if there are discussions going on) and do not approve your own changes. The idea is that someone else reviews the changes as merging automatically makes packages available in myget. This helps avoiding cleanup PRs like this one.

[nitpick] Use the imperative mood in the subject line of commit messages. See for example https://www.freecodecamp.org/news/how-to-write-better-git-commit-messages/

@emtuls emtuls changed the title Cleaned up psnotify chocoinstall Clean up psnotify chocoinstall Oct 23, 2023
Install-ChocolateyShortcut -shortcutFilePath $shortcut -targetPath $executablePath -WorkingDirectory $executableDir
VM-Assert-Path $shortcut

return $executablePath
Copy link
Member

@Ana06 Ana06 Oct 25, 2023

Choose a reason for hiding this comment

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

This return should be removed. It was in the helper to use from the calling code (outside the helper). We can merge the PR after that.

Copy link
Member

@Ana06 Ana06 left a comment

Choose a reason for hiding this comment

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

Thanks @emtuls!

@Ana06 Ana06 merged commit 4d11dd5 into main Oct 31, 2023
6 checks passed
@mr-tz mr-tz deleted the psnotify-fix branch October 31, 2023 19:24
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.

2 participants