Skip to content
This repository has been archived by the owner on Feb 22, 2022. It is now read-only.

Remove constants from the feature plugin #112

Merged
merged 2 commits into from
May 6, 2020
Merged

Remove constants from the feature plugin #112

merged 2 commits into from
May 6, 2020

Conversation

audrasjb
Copy link
Contributor

@audrasjb audrasjb commented May 6, 2020 •
edited
Loading

Let's remove the previously introduced constants.

After few discussions, it was also decided to remove the feature’s constants. Indeed, constants are used in WordPress core for specific cases:

  • Very early use, before WP is loaded
  • For use mostly by hosting companies/low level settings
  • Mostly for things that are really “constant” (never change)

For reference, see the following meeting recap.

Fixes #110

@audrasjb audrasjb requested a review from pbiron May 6, 2020 16:52
@audrasjb audrasjb self-assigned this May 6, 2020
@audrasjb audrasjb added the Core Merge Prerequisite This code that will require an update before merging to core label May 6, 2020
@audrasjb audrasjb added this to the 0.7.0 milestone May 6, 2020
whyisjake
whyisjake approved these changes May 6, 2020
View reviewed changes
Copy link
Member

@whyisjake whyisjake left a comment

Choose a reason for hiding this comment

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

lgtm

pbiron
pbiron suggested changes May 6, 2020
View reviewed changes
Copy link
Contributor

@pbiron pbiron left a comment

Choose a reason for hiding this comment

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

After these 4 minor changes, looks great.

Also, once you merge this, can you please update #20 accordingly?

functions.php Outdated Show resolved Hide resolved
functions.php Outdated Show resolved Hide resolved
functions.php Outdated Show resolved Hide resolved
functions.php Outdated Show resolved Hide resolved
@audrasjb audrasjb requested a review from pbiron May 6, 2020 17:24
pbiron
pbiron approved these changes May 6, 2020
View reviewed changes
Copy link
Contributor

@pbiron pbiron left a comment

Choose a reason for hiding this comment

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

merge away!

Don't forget to update #20 accordingly.

@audrasjb
Copy link
Contributor Author

audrasjb commented May 6, 2020

Fixes #110

@audrasjb audrasjb merged commit de1f009 into master May 6, 2020
@lukecav lukecav mentioned this pull request May 6, 2020
Open
@tangrufus tangrufus mentioned this pull request May 31, 2020
Open
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@whyisjake whyisjake whyisjake approved these changes

@pbiron pbiron pbiron approved these changes

Assignees

@audrasjb audrasjb

Labels
Core Merge Prerequisite This code that will require an update before merging to core
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.

Remove new constants
3 participants
@audrasjb @whyisjake @pbiron