-
Notifications
You must be signed in to change notification settings - Fork 10
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
(proposal): renovate to version 3 #28
base: develop
Are you sure you want to change the base?
Conversation
- Upgrades deps to latest. - Abstracts concerns to own files. - Implements new method signature for lookup and bulkLookup to allow optional params. - Implements dotenv with autoconfig for api key. - Exports types for "lru-cache". - Renovates tests. - Add typed errors. - Implements bin call via "lookup". - Update package.json to point to correct github repo. - Update readme.
Reference #27 |
bin/lookup.cmd
Outdated
@@ -0,0 +1,36 @@ | |||
::[Bat To Exe Converter] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this file do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a lets Windows run the binary node call, I'm not sure if that header part is even required tbh, I lifted it from an OCLIF auto-generated cmd file, but looking at their source it doesn't seem needed:
https://github.com/oclif/oclif/blob/master/bin/run.cmd
Likely just an artifact of whatever tool generates the cmd file in their build process so can be removed.
bin/lookup
Outdated
const args = process.argv.slice(2) | ||
const { IPData } = require("../lib/ipdata") | ||
const ip = args[0] || undefined | ||
const apiKey = args[1] || process.env.APIDATA_API_KEY || undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is process.env.APIDATA_API_KEY
correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, no - I hadn't actually gone through with final checks until I felt out if the shape of the PR was something you were interested in, or thought should fundamentally change etc -
Fixes #26 |
(PS: Probably worth flagging the PR as Draft until ready - apologies for not setting that from the start) https://github.blog/changelog/2020-04-08-convert-pull-request-to-draft/ Adding fixes:
|
- Fix incorrect env key. - Remove artefacts from Windows cmd bin. - Export shaped type for LRU cache. - Add comments. - Attempt to resolve constants from env before using fallbacks.
axios
(high security alert on current),url-join
,lodash
.debug
,got
(Axios replacement),dotenv
.debug