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

Allow defining custom locations for xhprof_lib and config.php #118

Merged
merged 1 commit into from Aug 15, 2018

Conversation

MarkRose
Copy link

The current code requires that config.php be located under the repository tree in a fixed location. We would like to manage it in another location.

Additionally, fix callgraph.php to load config.php from a custom xhprof_lib location.

@aik099
Copy link
Collaborator

aik099 commented Aug 14, 2018

I like the idea, but there is much code duplication.

Maybe we can define all the constants fallbacks in xhprof_lib/defaults.php file, that is then required in all 3 files, that you've changed?

@MarkRose MarkRose force-pushed the custom-xhprof-locations branch 2 times, most recently from 9ba42d3 to 23ec2b6 Compare August 14, 2018 20:25
@MarkRose
Copy link
Author

@aik099 I've updated my PR as requested.

@aik099
Copy link
Collaborator

aik099 commented Aug 15, 2018

  1. Ah, I though inclusion of config.php would stay in it's place. Now it's in defaults.php which needs to be renamed into something else to indicate, that it includes config.
  2. What is your use case, where at the same time you can't put your config.php file into the xhprof folder, but can include externals/header.php from it?

@MarkRose
Copy link
Author

  1. What would you like it to be named? Or, would you like me to move the code back into the three files?
  2. We're including xhprof as a git submodule. Also, as part of our workflow, we regularly blow away our code directory and deploy it into different environments with different configurations. We keep our configuration values outside of the code directory so it's simple to deploy new code.

@aik099
Copy link
Collaborator

aik099 commented Aug 15, 2018

  1. What would you like it to be named? Or, would you like me to move the code back into the three files?

Place is ok, just filename needs to be changed. Even though that might look like a step back, but please do this:

  1. keep filename as-is
  2. move out actual require ... XHPROF_CONFIG; line after the include of defaults.php happens
  1. We're including xhprof as a git submodule. Also, as part of our workflow, we regularly blow away our code directory and deploy it into different environments with different configurations. We keep our configuration values outside of the code directory so it's simple to deploy new code.

Sounds reasonable.

…faults file.

This change allows defining a custom location for XHPROF_LIB and
the config.php file.
@MarkRose
Copy link
Author

Changes made as requested.

@aik099 aik099 merged commit 2302bf7 into preinheimer:master Aug 15, 2018
@aik099
Copy link
Collaborator

aik099 commented Aug 15, 2018

Merging, thanks @MarkRose .

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.

None yet

3 participants