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

mp3tag: Update the manifest #14654

Merged
merged 9 commits into from
Dec 29, 2024
Merged

Conversation

FlawlessCasual17
Copy link
Contributor

@FlawlessCasual17 FlawlessCasual17 commented Dec 29, 2024

  • Changed the context menu option to use registry files.

  • Changed the pre_install and pre_uninstall scripts, so the explorer will no longer restart after either.

  • Added a post_install script.

  • I have read the Contributing Guide.

NOTE: This PR also updates some reg files for VS Code because someone forgot to end them with a blank new line, as seen here in this GitHub Actions log.

Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

mp3tag

  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

@FlawlessCasual17
Copy link
Contributor Author

@rasa

@rasa rasa merged commit eeea66b into ScoopInstaller:master Dec 29, 2024
2 checks passed
@glektarssza
Copy link

Did anyone actually test this? The changes for mp3tag do not work and are clearly wrong. L51 of bucket/mp3tag.json has a reference to *MyScoop* which is not a known replacement pattern of any kind.

image

Additionally, running uninstall spits out a bunch of errors from the reg commands if the install-mp3tag-content commands have not been run yet which is definitely not good behavior.

image

rasa added a commit that referenced this pull request Feb 3, 2025
@rasa
Copy link
Member

rasa commented Feb 3, 2025

@glektarssza Thank you for your detailed report. I did not test the changes, but I did review them, and they looked good. My apologies for any pain caused. I have reverted these changes, and will be more vigilant in the future.

@FlawlessCasual17
Copy link
Contributor Author

FlawlessCasual17 commented Feb 3, 2025

Hey @rasa, @glektarssza, sorry about this error, I was caught with other things.

I'll quickly make another PR, and I hope Rasa can merge it.

@FlawlessCasual17
Copy link
Contributor Author

I have made a new PR, #14834

@glektarssza
Copy link

Thank you both (@rasa and @FlawlessCasual17) for the quick response and turn around on getting this resolved. Kudos to both of you on that. This is honestly why I love scoop and will continue to use it over other package management systems for Windows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants