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.

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 Custom form defined #125 #130

Merged
merged 11 commits into from
Dec 16, 2019
Merged

Add Custom form defined #125 #130

merged 11 commits into from
Dec 16, 2019

Conversation

0xDkd
Copy link
Contributor

@0xDkd 0xDkd commented Dec 13, 2019

see #125

@0xDkd
Copy link
Contributor Author

0xDkd commented Dec 13, 2019

测试已经通过,不过我还是建议将 formMemformFile 改成 from
另外如果可以 merge 我下次吧对应部分的 example 和 readme 也修改一下

感谢您的开源,我学到了很多知识!

@codecov-io
Copy link

codecov-io commented Dec 13, 2019

Codecov Report

Merging #130 into master will increase coverage by 0.31%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage    92.8%   93.12%   +0.31%     
==========================================
  Files          32       32              
  Lines        1348     1396      +48     
==========================================
+ Hits         1251     1300      +49     
+ Misses         64       63       -1     
  Partials       33       33
Impacted Files Coverage Δ
utils.go 92% <ø> (ø) ⬆️
core/core.go 89.47% <ø> (ø) ⬆️
encode/form.go 89.05% <100%> (+5.53%) ⬆️
encode/encode_core.go 100% <100%> (ø) ⬆️
bench/task.go 87.8% <0%> (+1.21%) ⬆️

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 39593dc...24b85fc. Read the comment docs.

@@ -6,6 +6,7 @@
*.dll
*.so
*.dylib
.DS_Store
Copy link
Owner

Choose a reason for hiding this comment

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

忽略Ds_Store用途是?

.gitignore Show resolved Hide resolved
_example/go.mod Outdated
@@ -6,3 +6,5 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.1 // indirect
)

go 1.13
Copy link
Owner

Choose a reason for hiding this comment

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

这里我建议不加go 1.13。

@0xDkd
Copy link
Contributor Author

0xDkd commented Dec 14, 2019

用Mac写代码系统会自动生成.Ds_Store

另外go13可能是编辑器自动格式化生成的。

我修改一下

FileName string
ContentType string
File interface{}
}{FileName: "voice.pem", ContentType: "", File: "../testdata/voice.pcm"},
Copy link
Owner

@guonaihong guonaihong Dec 14, 2019

Choose a reason for hiding this comment

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

这里可以使用core.FileType结构体,代码会少写点。

Voice2: core.FormType{FileName: "voice.pem", ContentType: "", File: "../testdata/voice.pcm"}

FileName string
ContentType string
File interface{}
}{FileName: "voice.pem", ContentType: "", File: "pcm1"},
Copy link
Owner

Choose a reason for hiding this comment

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

也改成core.FormType

FileName string
ContentType string
File interface{}
}{FileName: "voice.pem", ContentType: "", File: core.FormMem("pcm1")},
Copy link
Owner

Choose a reason for hiding this comment

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

改成core.FormType

FileName string
ContentType string
File interface{}
}{FileName: "voice.pem", ContentType: "", File: "pcm1"},
Copy link
Owner

Choose a reason for hiding this comment

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

改成FormType

FileName string
ContentType string
File interface{}
}{FileName: "voice.pem", ContentType: "", File: "../testdata/voice.pcm"},
Copy link
Owner

Choose a reason for hiding this comment

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

改成FormType

File interface{}
}{FileName: "voice.pem", ContentType: "", File: "../testdata/voice.pcm"},
},
},
Copy link
Owner

Choose a reason for hiding this comment

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

测试正确的情况可以把结构体内嵌的case加上。比如把FormType内嵌到一个业务结构体里面。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个没有明白是什么意思,能麻烦您举个例子吗😂

Copy link
Owner

Choose a reason for hiding this comment

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

把下面的结构体用起来,看能否处理结构体内嵌的情况。

type test_Form_2_struct struct {
	Mode   string        `form:"mode"`
	Text   string        `form:"text"`
	core.FormType `form:"voice" form-mem:"true"` //内嵌core.FormType
	Url string `form:"url"`
}

0xDkd pushed a commit to 0xDkd/gout that referenced this pull request Dec 14, 2019
@guonaihong
Copy link
Owner

代码测试覆盖度有点下降,可以使用下面两条命令观查并提升。

make test
go tool cover -html=coverage.out

@0xDkd
Copy link
Contributor Author

0xDkd commented Dec 14, 2019

感谢指导,终于合格了😭

另外请您推荐一点学习 Go 的资料可以吗,我感觉您的代码让我学到了很多

@guonaihong
Copy link
Owner

go的话,就 go程序设计语言

encode/form.go Outdated
@@ -28,7 +31,7 @@ func toBytes(val reflect.Value) (all []byte, err error) {
case []byte:
all = v
default:

fmt.Println(val.Kind())
Copy link
Owner

Choose a reason for hiding this comment

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

这句打印的目的是?

@guonaihong
Copy link
Owner

guonaihong commented Dec 15, 2019

最后在测试case 加下结构体内嵌的情况,这个pr基本就可以。

type test_Form_2_struct struct {
	Mode   string        `form:"mode"`
	Text   string        `form:"text"`
	core.FormType `form:"voice" form-mem:"true"` //内嵌core.FormType
	Url string `form:"url"`
}

@0xDkd
Copy link
Contributor Author

0xDkd commented Dec 15, 2019

已经加上了,另外如果有什么任务也可以分给我

@guonaihong
Copy link
Owner

guonaihong commented Dec 15, 2019

下一个可以玩下。#128。pr明天早上我再过下,没问题就合并

encode/form.go Outdated
if !okToo {
return fmt.Errorf("unknown type formFileWrite:%T, openFile:%t", v, openFile)
}
content = []byte(s)
Copy link
Owner

Choose a reason for hiding this comment

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

这里可以使用core. StringToBytes转换。
在go里面[]byte(s)会分配内存。

encode/form.go Show resolved Hide resolved
@@ -156,6 +156,13 @@ func encode(val reflect.Value, sf reflect.StructField, a Adder) error {

typ := val.Type()

if strings.HasSuffix(typ.Name(), "FormType") {
if err := parseTagAndSet(val, sf, a); err != nil {
Copy link
Owner

Choose a reason for hiding this comment

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

这里可以用 return parseTagAndSet(val, sf, a)替换

@guonaihong guonaihong changed the title Add Custom form defined https://github.com/guonaihong/gout/issues/125 Add Custom form defined #125 Dec 16, 2019
@guonaihong guonaihong merged commit 3291333 into guonaihong:master Dec 16, 2019
@guonaihong guonaihong mentioned this pull request Dec 16, 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