Skip to content
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.

Sign up for GitHub

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

Jump to bottom

引入新请求类型，修复「添加企业客户标签」功能 #126

Merged
merged 1 commit into from
Dec 14, 2022
Merged

引入新请求类型，修复「添加企业客户标签」功能 #126

merged 1 commit into from
Dec 14, 2022

Conversation

rudy-tao
Copy link
Contributor

@rudy-tao rudy-tao commented Dec 9, 2022

新增omitempty，防止请求时报空字符错误

@rudy-tao
Copy link
Contributor Author

rudy-tao commented Dec 9, 2022

合并后麻烦加个tag

@rudy-tao rudy-tao closed this Dec 9, 2022
@rudy-tao rudy-tao reopened this Dec 9, 2022
@rudy-tao
Copy link
Contributor Author

hi~ 这个mr有问题嘛 我看一直没有回复or合并 @xen0n

@xen0n
Copy link
Owner

xen0n commented Dec 12, 2022

hi~ 这个mr有问题嘛 我看一直没有回复or合并 @xen0n

Hi，不好意思前面在做别的开源项目。

这里目前是直接把 doc md 中的 JSON 列的语义当成 Go 的 struct tag 了，严格意义上属于实现细节泄漏，不过目前因为 sdkcodegen 没拆成单独开源项目，所以问题可能也不大

主要就是我想不明白为啥以前没问题的调用，现在就不行了？“空字符错误”具体是个啥我也不清楚。如果有可能的话补个测试或者至少说明一下想做的事情，预期的结果，实际的响应会比较好。

@rudy-tao
Copy link
Contributor Author

hi~ 这个mr有问题嘛 我看一直没有回复or合并 @xen0n

Hi，不好意思前面在做别的开源项目。

这里目前是直接把 doc md 中的 JSON 列的语义当成 Go 的 struct tag 了，严格意义上属于实现细节泄漏，不过目前因为 sdkcodegen 没拆成单独开源项目，所以问题可能也不大

主要就是我想不明白为啥以前没问题的调用，现在就不行了？“空字符错误”具体是个啥我也不清楚。如果有可能的话补个测试或者至少说明一下想做的事情，预期的结果，实际的响应会比较好。

我的需求需要用到添加企业客户标签这个接口，此接口若有key但值为空的话，会报错unexpected empty string

@xen0n
Copy link
Owner

xen0n commented Dec 13, 2022

hi~ 这个mr有问题嘛 我看一直没有回复or合并 @xen0n

Hi，不好意思前面在做别的开源项目。

这里目前是直接把 doc md 中的 JSON 列的语义当成 Go 的 struct tag 了，严格意义上属于实现细节泄漏，不过目前因为 sdkcodegen 没拆成单独开源项目，所以问题可能也不大

主要就是我想不明白为啥以前没问题的调用，现在就不行了？“空字符错误”具体是个啥我也不清楚。如果有可能的话补个测试或者至少说明一下想做的事情，预期的结果，实际的响应会比较好。

我的需求需要用到添加企业客户标签这个接口，此接口若有key但值为空的话，会报错unexpected empty string

我知道了，此处不该复用结构体。。定义一个专门的小结构体就行了。不用加omitempty...

@rudy-tao
Copy link
Contributor Author

不行的 有些参数是选填，如果真的需要有值的话，这些key还是要带上的，所以我加了omitempty

@xen0n
Copy link
Owner

xen0n commented Dec 13, 2022

不行的 有些参数是选填，如果真的需要有值的话，这些key还是要带上的，所以我加了omitempty

又看了下文档，有些懂了，这接口里很多字段说的都是“必填：否”，这时候传一个空值和不传确实语义不同。

有个比较难顶的地方在于，如果不使用指针类型或者额外 flags，Go 无论在编译时还是运行时都无法区分传一个空值和不传值两种情况。我们是否有必要弄一个 ReqExternalContactAddCorpTag 单独类型来表达这种？否则比方说 order = 0 就无法表示。否则用同一个类型的话，必须涉及向公有类型加 omitempty 改变序列化语义，严格意义上是 breaking change，我就要切 v2 了。阁下如何评价？

@rudy-tao
Copy link
Contributor Author

不行的 有些参数是选填，如果真的需要有值的话，这些key还是要带上的，所以我加了omitempty

又看了下文档，有些懂了，这接口里很多字段说的都是“必填：否”，这时候传一个空值和不传确实语义不同。

有个比较难顶的地方在于，如果不使用指针类型或者额外 flags，Go 无论在编译时还是运行时都无法区分传一个空值和不传值两种情况。我们是否有必要弄一个 ReqExternalContactAddCorpTag 单独类型来表达这种？否则比方说 order = 0 就无法表示。否则用同一个类型的话，必须涉及向公有类型加 omitempty 改变序列化语义，严格意义上是 breaking change，我就要切 v2 了。阁下如何评价？

赞同，确实需要新建一个struct，学到了 感谢 @xen0n
我再改一下

xen0n
xen0n reviewed Dec 14, 2022
View reviewed changes
Copy link
Owner

@xen0n xen0n left a comment

Choose a reason for hiding this comment

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

改完之后把提交 squash 成一个应该就可以了

apis.md.go Show resolved Hide resolved
@rudy-tao rudy-tao force-pushed the fix/unexpected_empty_string branch 5 times, most recently from 5b2e62d to b26b7b6 Compare December 14, 2022 05:35
@rudy-tao
Copy link
Contributor Author

提交已squash成一个 @xen0n

@rudy-tao rudy-tao force-pushed the fix/unexpected_empty_string branch from f9809fd to 082e8dc Compare December 14, 2022 05:41
@rudy-tao
fix invalid group id unexpected empty string
2068419 
fix invalid group id unexpected empty string

build(deps): bump github.com/urfave/cli/v2 from 2.23.6 to 2.23.7

Bumps [github.com/urfave/cli/v2](https://github.com/urfave/cli) from 2.23.6 to 2.23.7.
- [Release notes](https://github.com/urfave/cli/releases)
- [Changelog](https://github.com/urfave/cli/blob/main/docs/CHANGELOG.md)
- [Commits](urfave/cli@v2.23.6...v2.23.7)

---
updated-dependencies:
- dependency-name: github.com/urfave/cli/v2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

fix invalid group id unexpected empty string

build(deps): bump github.com/urfave/cli/v2 from 2.23.6 to 2.23.7

Bumps [github.com/urfave/cli/v2](https://github.com/urfave/cli) from 2.23.6 to 2.23.7.
- [Release notes](https://github.com/urfave/cli/releases)
- [Changelog](https://github.com/urfave/cli/blob/main/docs/CHANGELOG.md)
- [Commits](urfave/cli@v2.23.6...v2.23.7)

---
updated-dependencies:
- dependency-name: github.com/urfave/cli/v2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

add omitempty

fix invalid group id unexpected empty string

fix go mod

fix
@rudy-tao rudy-tao force-pushed the fix/unexpected_empty_string branch from 082e8dc to 2068419 Compare December 14, 2022 05:42
@xen0n xen0n changed the title fix invalid group id unexpected empty string 引入新请求类型，修复「添加企业客户标签」功能 Dec 14, 2022
@xen0n
Copy link
Owner

xen0n commented Dec 14, 2022

帮你改了 PR 标题，今天下班前会有一个新 tag，不 bump 大版本（因为按照当前改动的性质，先前这个接口想必是不能正常使用的，所以能够假定不会存在野生代码依赖了先前的方法签名）

十分感谢贡献！

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 14, 2022

Build succeeded:

  • build

@bors bors bot merged commit 5e7f87a into xen0n:develop Dec 14, 2022
@xen0n
Copy link
Owner

xen0n commented Dec 14, 2022

合并后麻烦加个tag

tag v1.3.0 pushed

请慢用！

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xen0n xen0n xen0n left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rudy-tao @xen0n