Skip to content
This repository has been archived by the owner on Aug 4, 2023. It is now read-only.

docs: improve README.md #4

Merged
merged 1 commit into from
Jul 17, 2018
Merged

docs: improve README.md #4

merged 1 commit into from
Jul 17, 2018

Conversation

watson
Copy link
Contributor

@watson watson commented Jul 15, 2018

No description provided.

@watson watson self-assigned this Jul 15, 2018
@watson watson requested a review from Qard July 15, 2018 11:22
@@ -20,7 +21,7 @@ branch](https://github.com/elastic/apm-nodejs-http-client/tree/opbeat).
## Installation

```
npm install elastic-apm-http-client
npm install elastic-apm-http-client --save
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a particular reason for adding --save? It's the default behavior, so I don't think we need that. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old habit I guess. I have disabled automatic saving in my personal .npmrc because it's often in the way (I often install a lot of modules for debugging purposes, and I don't want those added to the package.json). But I'm probably the odd one out. If we decide not to have --save here, we should probably also remove it from the agent docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with including it. I just didn't really get why. I don't disable it, personally. I just use --no-save when installing something for debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After going back and forth on this for a bit, I think I'd like to keep it. Even though it's now the default behavior, it signals to the reader that this module is intended to be installed as a dependency - as opposed to as a dev-dependency or globally or temporarily for debugging. I like being explicit 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. 👍

@watson watson merged commit e2250a9 into master Jul 17, 2018
@watson watson deleted the readme branch July 17, 2018 09:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants