-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add client of Marketing domain to handle marketing email statistics #6
Conversation
ttyfky
commented
May 13, 2022
- Add marketing service
- Add API of marketing email statistics
- Add legacy package to handle HubSpot API v1 context
- Add doc.go in the root and legacy package
- Add Marketing service - Add API of marketing email statistics - Add legacy package to handle HubSpot API v1 context - Add doc.go in the root and legacy package
@@ -16,7 +17,7 @@ type ExampleContact struct { | |||
} | |||
|
|||
func ExampleContactServiceOp_Create() { | |||
cli, _ := hubspot.NewClient(hubspot.SetAPIKey("apikey")) | |||
cli, _ := hubspot.NewClient(hubspot.SetAPIKey(os.Getenv("API_KEY"))) |
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.
Made apikey to env var to prevent upload real apikey after testing.
- Add Marketing service - Add API of marketing email statistics - Add legacy package to handle HubSpot API v1 context - Add doc.go in the root and legacy package fix: fix test by adding ExportNewMarketing fix: set dmmmy emailID fix: improve struct structure
Marketingの分のテストはどのタイミングで足しますか?(後で僕の方で足すでも大丈夫です) |
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.
いくつかコメントしました
(構造体のフィールド名は自動生成だと思うので特に付けてません)
あとついでに README.md の Available list も更新して貰えると助かります。。 |
Co-authored-by: kei <49366645+kk-no@users.noreply.github.com>
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.
- Fixed the code as per comment
- Add a query option for a bulk operation
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.
LGTM