Skip to content

Conversation

@prescience-data
Copy link

@prescience-data prescience-data commented Mar 26, 2021

  • Upgrades deps to latest.
  • Removes deps for axios (high security alert on current), url-join, lodash.
  • Adds deps on debug, got (Axios replacement), dotenv.
  • 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".
  • Adds scoped debug logs via debug
  • Renovates tests.
  • Add typed errors.
  • Implements bin call via "lookup".
  • Update package.json to point to correct github repo.
  • Update readme.

- 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.
@prescience-data
Copy link
Author

Reference #27

bin/lookup.cmd Outdated
@@ -0,0 +1,36 @@
::[Bat To Exe Converter]
Copy link
Collaborator

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?

Copy link
Author

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
Copy link
Collaborator

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?

Copy link
Author

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 -

@thomasconner
Copy link
Collaborator

Fixes #26

@prescience-data
Copy link
Author

prescience-data commented Mar 31, 2021

(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 artifacts from Windows cmd bin.
  • Export shaped type for LRU cache.
  • Add comments.
  • Attempt to resolve constants from env before using fallbacks.

- 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.
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.

2 participants