Skip to content

Conversation

@acoshift
Copy link
Contributor

Thank you for the hard work for Line Bot SDK for Go 👍

This PR changes

  • Implement MarshalJSON for Flex Components, and Containers
  • Breaking Change Remove field Type from Flex Components, and Containers

I think it very confusing to put the same Type to the named struct.

@sugyan sugyan self-requested a review November 27, 2018 08:32
Copy link
Contributor

@sugyan sugyan left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.
But... we don't assume much the use case of writing each struct of containers and components. It is convenient that use UnmarshalJSON method from JSON data which is generated by the simulator.
https://developers.line.biz/console/fx/
So, we think that it is not needed to implement MashalJSON.

Or how about adding only the implementation of MarshalJSON so as not to make a destructive change?
For example, without deleting the definition of the Type element, when the correct value is not set, MarshalJSON automatically complements it inside it.

@acoshift
Copy link
Contributor Author

Hi,
I change the PR to implement only MarshalJSON without breaking change as your suggest.

My use-case is to generate flex message dynamically from backend.
I think it's harder to unmarshal from json and replace data/elements.

@sugyan sugyan requested a review from k2wanko November 28, 2018 04:04
Copy link
Contributor

@k2wanko k2wanko left a comment

Choose a reason for hiding this comment

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

LGTM

@sugyan sugyan merged commit 8a7ff77 into line:master Nov 28, 2018
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