-
Notifications
You must be signed in to change notification settings - Fork 255
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
Update README to include configuration options #162
Conversation
hapikey: <HAPIKEY>, | ||
base_url: "https://api.hubapi.com", | ||
portal_id: <PORTAL_ID>, | ||
logger: Logger.new(nil), |
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.
I'm not certain why this defaults to nil
. I think we want this to default to Logger.new(STDOUT)
.
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.
I think defaulting to nil means that it isn't logging which may be a reasonable default if this is noisy.
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.
1 suggestion otherwise this seems like a great change 👍
Below is a complete list of configuration options with the default values: | ||
```ruby | ||
Hubspot.configure({ | ||
hapikey: <HAPIKEY>, |
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.
I've just opened a PR on the node side adding these to the readme with values that are closer to reality. Do you think it's worth doing something similar here? MadKudu/node-hubspot#138
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.
Your README updates in the node project look excellent 🌟 I think this README would benefit from additional information around the "contributing" and "testing" sections. The list of configuration options here is more for the API users rather than a contributor.
This idea is based on issue #18 where a user had trouble understanding how to set the portal_id
value.
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.
I do like how you've included examples of the access token and api key. I worry if I add those values here, it will conflict with showing the default values for base_url
and logger
. I'm going to defer adding that for now and add a similar example when updating the contributing section.
Summary: Discovering the configuration options requires a bit of sleuthing. To help our users, this lists the configuration options on the README in addition to providing some helpful links to the HubSpot API Docs.
b6aa0b5
to
fb529f0
Compare
Address Issue: #18
Summary:
Discovering the configuration options requires a bit of sleuthing. To
help our users, this lists the configuration options on the README in
addition to providing some helpful links to the HubSpot API Docs.