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

Use package.json information to set user-agent string #57

Closed
wants to merge 2 commits into from
Closed

Use package.json information to set user-agent string #57

wants to merge 2 commits into from

Conversation

nbusseneau
Copy link

@nbusseneau nbusseneau commented Aug 19, 2019

Warning: requires #56 to be merged.

This PR automates user-agent string with information coming from package.json. Be aware that I have absolutely no idea how the following line behaves on environments that are not node.js:

const packageJson = require('../package.json');

It works completely fine on node.js because package.json can be reached from node_modules, but if this script is to be used in projects not using node_modules or not including package.json it will not work.

Do such projects even exist? Should this library cater to them? I'll let you decide because I have very minimal experience with the whole JS/NPM stack and basically don't know what is possible or not.

Just having the headers seems to be enough, the actual data is ignored. See #53 for discussion.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 62.055% when pulling 1637f19 on Skymirrh:feature/better-user-agent into 5280d9a on tdurieux:master.

@tdurieux
Copy link
Owner

I always focused on pure nodejs because leboncoin does not allow cross-domain requests thus this library would not work in the browser.

@nbusseneau
Copy link
Author

Closing since this code is now obsolete following latest changes. It may come back in a different flavour depending on the outcome of the discussion in #53.

@nbusseneau nbusseneau closed this Aug 21, 2019
@nbusseneau nbusseneau deleted the feature/better-user-agent branch August 21, 2019 08:38
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.

3 participants