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

Use const instead of define everywhere it can be applicable #70

Closed
markkap opened this Issue Sep 22, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@markkap
Copy link

markkap commented Sep 22, 2018

Salts and DB access info in wp-config.php are probably good starting point.

The advantage performance wise might be minute as the main difference is that consts can be cached by the php interpreter while defines has to be evaluated at run time. The bigger advantage is readability as with defines is not always trivial to find where a value might be defined

@markkap markkap added the performance label Sep 22, 2018

@markkap markkap added this to the 1.0.0 milestone Sep 22, 2018

@mikeschinkel

This comment has been minimized.

Copy link

mikeschinkel commented Oct 30, 2018

If we could use an approach that would allow them to be reset then it would make unit testing much easier. Neither defines nor consts can be redefined, but properties could be "locked" and then later "unlocked" when they need to be reset.

Something to discuss?

@markkap

This comment has been minimized.

Copy link
Author

markkap commented Nov 5, 2018

@mikeschinkel thanks for the discussion about having local enviroment overrides I realized that the use of const will just prevent local overrides. With all the PHP language based advantages the const construct has, it is just not flexible enough to allow overrides, so I guess define is the only way to go.

Will close this one, and open an "scifi" issue describing how configs might work IMO.

@markkap markkap closed this Nov 5, 2018

@mikeschinkel

This comment has been minimized.

Copy link

mikeschinkel commented Nov 5, 2018

@markkap For the past week I have been working on something I felt was best described as "Better WP-Config." If can work with existing WordPress, and/or it could also be used by CalmPress to eradicate all define()d constants from core, if you are up for doing that.

I started working on it because of the potential for something better in CalmPress (and ClassicPress) and so I brainstormed all the things I found wrong with the current wp-config.php and think I have identified and implemented a solution that addresses all issues I could think of.

The code is done (except for the final feature to allow 3rd party providers to extend it) but I plan to document it before I implement that so I can share it with you and others.

I assume you will be interested in reviewing it and giving constructive feedback?

@markkap

This comment has been minimized.

Copy link
Author

markkap commented Nov 5, 2018

@mikeschinkel

This comment has been minimized.

Copy link

mikeschinkel commented Nov 5, 2018

@markkap Yes, very familiar with Bedrock.

Bedrock's approach is certainly nicer than WordPress' defaults but it does not really address a lot of the issues I've identified when working with WPLib Box and trying to deploy to WPEngine and Pantheon, and that is the need to have a configuration strategy that can work locally, on any environment and that the webhost would be able to incorporate without having to write a whole a new custom config that pushes the burden of working through all the issues related to config onto every team so that every web host and every team ends up rolling their own custom configuration solution.

What I have am working on is intended to be something that could support every web host, every 3rd party plugin and every use-case without even team and every webhost having to roll their own incompatible solution.

BTW, it can use phpdotenv or JSON (not recommended) or just PHP (recommended.) That said, it could conceivably feed Bedrock's configuration system too.

I want to share what I have with you but without sufficient docs I think I would be doing us both a disservice.

@mikeschinkel

This comment has been minimized.

Copy link

mikeschinkel commented Nov 5, 2018

@markkap Okay, I was able to get sufficient docs online to share it with you:

It is a general-purpose solution for WordPress, not something I was intending to be CalmPress only, so I think I would like to promote for use by all of the WordPress community assuming it does meet many people's needs as I envision it will.

So if you are willing it would be great to get feedback on it either via our Slack (join here) and/or via our GitHub issues rather than have this ticket turn into discussions which could easily turn into many tickets for Better WP-Config.

Thanks in advance.

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