Skip to content

Conversation

@TonPC64
Copy link
Contributor

@TonPC64 TonPC64 commented Oct 28, 2019

@codecov-io
Copy link

codecov-io commented Oct 28, 2019

Codecov Report

Merging #176 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #176      +/-   ##
==========================================
+ Coverage   73.41%   73.49%   +0.08%     
==========================================
  Files          24       24              
  Lines        1903     1909       +6     
==========================================
+ Hits         1397     1403       +6     
  Misses        423      423              
  Partials       83       83
Impacted Files Coverage Δ
linebot/imagemap.go 94.44% <100%> (+1.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b233d07...8cf5cb6. Read the comment docs.

func (a *URIImagemapAction) MarshalJSON() ([]byte, error) {
return json.Marshal(&struct {
Type ImagemapActionType `json:"type"`
Label string `json:"label"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The document says this field is optional. Can we include "omitempty" in this tag just like other Actions such as PostBackAction, URIAction, etc?

Text string `json:"text"`
Area ImagemapArea `json:"area"`
Type ImagemapActionType `json:"type"`
Label string `json:"label"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oklahomer
Copy link
Contributor

oklahomer commented Nov 16, 2019

I appreciate your effort to add go.mod. However, I believe a single pull request should focus on a single feature/issue. Especially when the new requirements include google.golang.org/appengine, which actually seems to be optional dependency to run examples/echo_bot_handler with build tag of -tags appengine.
So could you remove this change from this p-r? Other than that, those changes for labels look good.

@TonPC64
Copy link
Contributor Author

TonPC64 commented Nov 20, 2019

I appreciate your effort to add go.mod. However, I believe a single pull request should focus on a single feature/issue. Especially when the new requirements include google.golang.org/appengine, which actually seems to be optional dependency to run examples/echo_bot_handler with build tag of -tags appengine.
So could you remove this change from this p-r? Other than that, those changes for labels look good.

ok removed

Copy link
Contributor

@oklahomer oklahomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the additional changes 👍

@oklahomer oklahomer merged commit 1f51ed1 into line:master Nov 20, 2019
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