The Wayback Machine - https://web.archive.org/web/20210811103451/https://github.com/discourse/discourse/pull/13916
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: Remove PostProcessed trigger option #13916

Merged
merged 4 commits into from Aug 4, 2021
Merged

DEV: Remove PostProcessed trigger option #13916

merged 4 commits into from Aug 4, 2021

Conversation

@CvX
Copy link
Contributor

@CvX CvX commented Aug 2, 2021

It was deprecated 5 years ago in e55e2af

I've seen it still being used in the wild, even though it doesn't do anything anymore as I understand it.

@ZogStriP do you foresee any issues if 16 would eventually be reused as a trigger value? E.g. are badge seeds executed before post_migrate? (so the one added here would override them on new forums?) If so, would it be better to make it a regular migration? Or to leave a comment "reserving" that value in the enum?

(Also, it's weird we're using power of two values, since it's not a bit-field, i.e. only a single enum value can be active at a time)

It was deprecated 5 years ago in e55e2af
@CvX CvX requested a review from ZogStriP Aug 2, 2021
@eviltrout
Copy link
Member

@eviltrout eviltrout commented Aug 3, 2021

The person creating the enum might have thought at some point that a bit field made sense. Or perhaps we had one and migrated away from it but kept the values? Either way looks like we have to live with it.

I would personally advise against re-using the value. I would recommend renaming it to NoLongerUsed or something similar.

@ZogStriP
Copy link
Member

@ZogStriP ZogStriP commented Aug 3, 2021

I agree with @eviltrout, we should not allow this value to be reused anymore by assigning it to a "deprecated" variable.

@CvX CvX merged commit 07c6b72 into main Aug 4, 2021
9 checks passed
9 checks passed
@github-actions
core backend
Details
@github-actions
plugins backend
Details
@github-actions
core frontend
Details
@github-actions
plugins frontend
Details
@github-actions
core annotations
Details
@lgtm-com
LGTM analysis: JavaScript No new or fixed alerts
Details
license/cla Contributor License Agreement is signed.
Details
@CvX CvX deleted the drop-old-badge-trigger branch Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants