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

feat(request-node): Add Request Node version and Request Client version to requests header #192

Merged
merged 9 commits into from
Apr 14, 2020

Conversation

lumtis
Copy link
Contributor

@lumtis lumtis commented Apr 8, 2020

Description of the changes

Added the Request Node version in the response's header of each request to the Request Node.

Link to Jira

https://requestnetwork.atlassian.net/browse/PROT-1150

packages/request-node/src/requestNode.ts Outdated Show resolved Hide resolved
@lumtis lumtis changed the title feat(request-node): Add Request Node version to the response's header (WIP) feat(request-node): Add Request Node version to the response's header Apr 8, 2020
@lumtis lumtis requested a review from rittme April 8, 2020 07:40
@lumtis lumtis changed the title (WIP) feat(request-node): Add Request Node version to the response's header feat(request-node): Add Request Node version and Request Client version to requests header Apr 8, 2020
packages/request-node/src/requestNode.ts Outdated Show resolved Hide resolved
it('the response header contains the Request Node version', async () => {
// Import directly requestNode to create a server
requestNodeInstance = new requestNode();
server = requestNodeInstance.listen(3002, () => 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't have to use listen in unit tests, aren't we using supertest?

Copy link
Contributor

Choose a reason for hiding this comment

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

ping :)

@@ -4,6 +4,9 @@ import axios, { AxiosRequestConfig } from 'axios';

import { EventEmitter } from 'events';

const packageJson = require('../package.json');
const REQUEST_CLIENT_VERSION_HEADER = 'X-Request-Client-Version';
Copy link
Contributor

Choose a reason for hiding this comment

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

X-Request-Client-Version is this name not too generic?
maybe X-Request-Network-Client-Version to avoid collisions.
It's a genuine question maybe the name is already specific enough.

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 remark

yarn.lock Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Apr 9, 2020

Coverage Status

Coverage increased (+0.02%) to 90.439% when pulling 59cb8df on request-node-version-header into 5957961 on master.

@lumtis lumtis merged commit 20ad94b into master Apr 14, 2020
@lumtis lumtis deleted the request-node-version-header branch April 14, 2020 04:32
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

5 participants