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

Edits on Apollo Server metrics article #3180

Merged
merged 2 commits into from
Aug 21, 2019
Merged

Conversation

StephenBarlow
Copy link
Contributor

Holding off on modifying the Logging section of this article until #2008 lands.

Copy link
Member

@abernix abernix left a comment

Choose a reason for hiding this comment

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

I've left a couple comments within, but these are great improvements! Thank you!

There are two ways to configure client awareness:
> Client version is **not** tied to your current version of Apollo
> Client (or any other client library). You define this value and are responsible
> for updating it whenever meaningful changes are made to your client.
Copy link
Member

Choose a reason for hiding this comment

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

This is a great distinction to make!


```bash
# Replace the example values below with values specific to your use case.
ENGINE_API_KEY=YOUR_API_KEY ENGINE_SCHEMA_TAG=development node start-server.js
```

### Client awareness
### Identifying distinct client versions
Copy link
Member

Choose a reason for hiding this comment

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

For my own curiosity and understanding: Is the idea here to change the title to describe the user action rather than the feature?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct. This isn't an across-the-board change that we need to make or anything, but since "client awareness" isn't a universally known term, it felt appropriate to err on the side of the task instead of the feature in this case.


```bash
# Replace the example values below with values specific to your use case.
ENGINE_API_KEY=YOUR_API_KEY ENGINE_SCHEMA_TAG=development node start-server.js
```

### Client awareness
### Identifying distinct client versions
Copy link
Member

Choose a reason for hiding this comment

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

Since there's a specific configuration notion of client version below, my initial thought was that the inclusion of versions in this title was possibly too specific.

In other words, if Identifying distinct clients would be better.

Anyhow, definitely not asking for a change on this, just noting my instinct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! Will change.

@abernix
Copy link
Member

abernix commented Aug 21, 2019

Also, we should certainly land #2008 soon.

@StephenBarlow StephenBarlow merged commit 4d6306a into master Aug 21, 2019
@abernix abernix deleted the sb/metrics-edits branch February 25, 2020 21:03
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
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