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

DEV: Add notification_types table #8085

Conversation

@danielwaterworth
Copy link
Member

commented Sep 11, 2019

This table stores the ids of the notfication types so that notification
types can be added by plugins.

@discoursebot

This comment has been minimized.

Copy link

commented Sep 11, 2019

You've signed the CLA, danielwaterworth. Thank you! This pull request is ready for review.

DEV: Add notification_types table
This table stores the ids of the notfication types so that notification
types can be added by plugins.

@danielwaterworth danielwaterworth force-pushed the danielwaterworth:dynamic-notification-types branch from efca565 to f9db0d3 Sep 11, 2019

@SamSaffron

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

I am very mixed on this, the current approach of requiring plugins to submit a PR to core solves this. I don't really see anything hugely problematic with this approach.

It gets confusing if id 22 on one db means something and another something else. Current approach gives us parity across instances.

@eviltrout

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

There is a related discussion here:

#8077

This is a draft to demonstrate one approach. Personally I don't think it's worth the added complexity at this point. It's mostly solving a problem we don't have right now.

@eviltrout eviltrout closed this Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.