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

Have default export as a function #238

Closed
wants to merge 4 commits into from
Closed

Conversation

arnarthor
Copy link

I was looking at the code and found it odd that the Config.initialize() does take a config parameter but it's not used in ./index.js.

Therefor I'd like propose this change. I do realise that this is a breaking change for the package. Now instead of doing require('newrelic), it would need to be require('newrelic')().

This additionally allows for use of sending a config into the constructor of the package instead of having to rely on a file in the root of the project so projects could do something like require('newrelic')(config) which could then be located else where than in the root. I generally don't like having files located in the root due to the folder structure in projects I work on. I don't like having hard dependencies on where files are located as configurations to packages

If there's a specific reason for the other API feel free to close this. Since the config initialiser accepts the argument already this makes I think this could make sense.

@NatalieWolfe
Copy link
Contributor

Thanks for the patch, however I do not believe this is necessary. You can specify the path to the directory containing the configuration file using the environment variable NEW_RELIC_HOME. So, for instance if you have this project structure:

/path/to/project/
  - index.js
  - configs/
    - newrelic.js

You could set NEW_RELIC_HOME=/path/to/project/configs. In addition, you can set most configuration values through environment variables, so if you prefer you could use those entirely and set NEW_RELIC_NO_CONFIG_FILE=true then you do not need a newrelic.js file at all. Please let me know if either of these solutions work for you.

The reason Config.initialize accepts an optional configuration object is for our own testing, so that each test can provide its own configuration without needing separate configuration files.

@arnarthor
Copy link
Author

Ahh ok, fair enough. Didn't notice the setting for the home file. I'll close this then.

@arnarthor arnarthor closed this Jan 12, 2017
bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 2024
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

2 participants