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

feat: add DinkMigrate client command #564

Open
wants to merge 31 commits into
base: master
Choose a base branch
from

Conversation

@iProdigy iProdigy marked this pull request as ready for review October 11, 2024 17:08
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

The readme is clear & the code looks good to me. Should be a fairly simple review - builder pattern looks good for both the small & big plugins.

Just one question regarding the message after a migration, then I'm good

I have not tested this yet, which we should do somewhat thoroughly before merging this in (+ making sure maintainers have a chance to look at the issues you've created)

Comment on lines +208 to +209
plugin.addChatSuccess("Finished migrating configs from other plugins. " +
"Please verify the latest Dink settings and disable your other webhook plugins");
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention that you need to re-open config panel for changes to be reflected?

Copy link
Member Author

Choose a reason for hiding this comment

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

Please close and open the plugin settings panel for these changes to be visually reflected. should already be in chat from the handleImport call, but we could reiterate it since ppl struggle to read

@iProdigy iProdigy added this to the 1.10.13 milestone Oct 18, 2024
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.

Add support for migrating config from the Discord Rare Drop Notifier plugin
2 participants