Skip to content

feature: create rows columns and value from structs and slice by custom tag#280

Open
Rennbon wants to merge 7 commits into
DATA-DOG:masterfrom
Rennbon:master
Open

feature: create rows columns and value from structs and slice by custom tag#280
Rennbon wants to merge 7 commits into
DATA-DOG:masterfrom
Rennbon:master

Conversation

@Rennbon

@Rennbon Rennbon commented Dec 7, 2021

Copy link
Copy Markdown

It's easier to develop

@theodesp

theodesp commented Dec 7, 2021

Copy link
Copy Markdown
Collaborator

Hello @Rennbon. That is a great addition. I noticed that you add a new dependency for testing here. Do you think you can rewrite the test cases using the vanilla testing package only as I'm not sure if there is a need to change that at the moment?
@l3pp4rd do you agree?

@danpi

danpi commented Dec 7, 2021 via email

Copy link
Copy Markdown

@AlekSi

AlekSi commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

I wonder why you ask me :)

Personally, I do use testify; but if this project does not use it yet and has the policy to avoid external testing libraries, I agree that introducing it there is not a good idea.

@theodesp

theodesp commented Dec 7, 2021

Copy link
Copy Markdown
Collaborator

I wonder why you ask me :)

Personally, I do use testify; but if this project does not use it yet and has the policy to avoid external testing libraries, I agree that introducing it there is not a good idea.

Hey @AlekSi Sorry I tagged you wrongly. Glad to get some feedback though.

@Rennbon

Rennbon commented Dec 7, 2021

Copy link
Copy Markdown
Author

That is a great addition. I noticed that you add a new dependency for testing here. Do you think you can rewrite the test cases using the vanilla testing package only as I'm not sure if there is a need to change that at the moment?
@l3pp4rd do you agree?

OK

@Rennbon Rennbon closed this Dec 7, 2021
@Rennbon Rennbon reopened this Dec 7, 2021
@Rennbon

Rennbon commented Dec 7, 2021

Copy link
Copy Markdown
Author

@theodesp I removed testify and used reflect.DeepEqual to verify that it was as expected.

@Rennbon Rennbon force-pushed the master branch 4 times, most recently from 2e690de to 94ed2f6 Compare December 8, 2021 03:24
Author:    Rennbon <343688972@qq.com>
Author:    Rennbon <343688972@qq.com>
Comment thread rows.go Outdated
Comment thread rows_test.go
Comment thread rows.go Outdated
Comment thread rows.go Outdated
@Rennbon Rennbon requested a review from theodesp December 20, 2021 11:17
@theodesp

Copy link
Copy Markdown
Collaborator

@Rennbon Thank you. Will try to test it this week a bit more.

@Rennbon

Rennbon commented Dec 20, 2021

Copy link
Copy Markdown
Author

@Rennbon Thank you. Will try to test it this week a bit more.

Ok, hope to merge soon.

@Rennbon

Rennbon commented Feb 15, 2022

Copy link
Copy Markdown
Author

@theodesp Is there anything wrong with this PR

Comment thread rows_test.go Outdated
@theodesp

Copy link
Copy Markdown
Collaborator

@l3pp4rd Is there a way to add this PR in Travis CI/CD pipeline please. It seems that I'm not able to see the test results in order to merge. Thank you.

@dolmen

dolmen commented Apr 22, 2022

Copy link
Copy Markdown
Contributor

Those are utility functions. They don't have to be in the sqlmock core. They could be published as a separate package.

Comment thread rows.go
@Rennbon

Rennbon commented Apr 23, 2022

Copy link
Copy Markdown
Author

Those are utility functions. They don't have to be in the sqlmock core. They could be published as a separate package.

From a usage perspective, this commit reduces mock writing complexity.
If go-sqlmock implements this functionality directly, it will be easier for developers to use.

Comment thread rows.go
}
for i := 0; i < num; i++ {
f := val.Type().Field(i)
column := f.Tag.Get(tag)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code allows to use JSON tags, but doesn't fully support them.

Examples of incorrectly handled values: "-", "data,omitempty".

I think that using JSON tags is a bad default.

Comment thread rows.go
// NewRowsFromInterface new Rows from struct or slice or array reflect with tagName
// NOTE: arr/slice must be of the same type
// tagName default "json"
func NewRowsFromInterface(m interface{}, tagName string) (*Rows, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Go has generics now. It would be better to have multiple functions with stronger typing.

NewRowsFromSlice
NewRowsFromStruct

@Rennbon

Rennbon commented May 7, 2022

Copy link
Copy Markdown
Author

@dolmen 这个库是Go 1.15的,不是所有引用库都会升级到1.18的

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.

5 participants