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

add method ScanAndCount and AllAndCount for Model package gdb #2635

Merged
merged 1 commit into from
Jun 1, 2023
Merged

add method ScanAndCount and AllAndCount for Model package gdb #2635

merged 1 commit into from
Jun 1, 2023

Conversation

lusson-luo
Copy link
Contributor

feat: 添加方法两个 ScanAndCount, AllAndCount.

@lusson-luo lusson-luo mentioned this pull request May 9, 2023
Closed
@codecov-commenter
Copy link

codecov-commenter commented May 9, 2023 •
edited
Loading

Codecov Report

Patch coverage: 76.00% and project coverage change: -0.02 ⚠️

Comparison is base (4914517) 78.88% compared to head (29d744a) 78.87%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2635      +/-   ##
==========================================
- Coverage   78.88%   78.87%   -0.02%     
==========================================
  Files         631      631              
  Lines       51784    51809      +25     
==========================================
+ Hits        40849    40862      +13     
- Misses       8913     8921       +8     
- Partials     2022     2026       +4
Flag Coverage Δ
go-1.15-386 78.89% <76.00%> (-0.01%) ⬇️
go-1.15-amd64 78.88% <76.00%> (+<0.01%) ⬆️
go-1.16-386 ?
go-1.16-amd64 78.88% <76.00%> (-0.01%) ⬇️
go-1.17-386 78.87% <76.00%> (?)
go-1.17-amd64 78.87% <76.00%> (-0.04%) ⬇️
go-1.18-386 78.80% <76.00%> (-0.04%) ⬇️
go-1.18-amd64 78.82% <76.00%> (+0.02%) ⬆️
go-1.19-386 78.89% <76.00%> (?)
go-1.19-amd64 78.89% <76.00%> (-0.03%) ⬇️
go-1.20-386 78.83% <76.00%> (+0.48%) ⬆️
go-1.20-amd64 78.83% <76.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
database/gdb/gdb_model_select.go 72.22% <76.00%> (+0.20%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

houseme
houseme reviewed May 9, 2023
View reviewed changes
Copy link
Member

@houseme houseme left a comment

Choose a reason for hiding this comment

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

建议补上对应的单元测试代码

@houseme houseme requested review from gqcn and hailaz May 9, 2023 06:51
@lusson-luo
Copy link
Contributor Author

建议补上对应的单元测试代码

单元测试有例子吗，我看 Scan 和 All 之前没有单元测试，担心 orm 依赖数据库，可以不依赖数据库写单元测试吗？

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

It is recommended to add the corresponding unit test code

Are there any examples of unit tests? I see that Scan and All have no unit tests before. I am worried that orm depends on the database. Can you write unit tests without relying on the database?

@houseme
Copy link
Member

houseme commented May 9, 2023

建议补上对应的单元测试代码

单元测试有例子吗，我看 Scan 和 All 之前没有单元测试，担心 orm 依赖数据库，可以不依赖数据库写单元测试吗？

有安装 mysql 的数据库的，你直接模拟先写书 20 条，在按照条件去查询就可以了

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

It is recommended to add the corresponding unit test code

Is there an example of unit testing? I see that Scan and All have no unit testing before. I am worried that orm depends on the database. Can I write unit testing without relying on the database?

If you have a mysql database installed, you can directly simulate writing 20 books first, and then query according to the conditions

@lusson-luo
Copy link
Contributor Author

建议补上对应的单元测试代码

单元测试有例子吗，我看 Scan 和 All 之前没有单元测试，担心 orm 依赖数据库，可以不依赖数据库写单元测试吗？

有安装 mysql 的数据库的，你直接模拟先写书 20 条，在按照条件去查询就可以了

在 sqlite 上补充单元测试代码

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

It is recommended to add the corresponding unit test code

Is there an example of unit testing? I see that Scan and All have no unit testing before. I am worried that orm depends on the database. Can I write unit testing without relying on the database?

If you have a mysql database installed, you can directly simulate writing 20 books first, and then query according to the conditions

Supplement the unit test code on sqlite

@lusson-luo lusson-luo requested a review from houseme May 10, 2023 02:25
@lusson-luo
Copy link
Contributor Author

@gqcn 大佬审核下 pr

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@gqcn boss review the pr

gqcn
gqcn requested changes May 11, 2023
View reviewed changes
contrib/drivers/sqlite/sqlite_model_test.go Show resolved Hide resolved
contrib/drivers/sqlite/sqlite_model_test.go Show resolved Hide resolved
database/gdb/gdb_model_select.go Outdated Show resolved Hide resolved
database/gdb/gdb_model_select.go Outdated Show resolved Hide resolved
@gqcn
Copy link
Member

gqcn commented May 12, 2023

@lusson-luo The feature is awesome, but the CI failed, please have a check.

@lusson-luo lusson-luo force-pushed the feat/addCountQuery branch 2 times, most recently from 0f40ad1 to 69d01a8 Compare May 15, 2023 02:51
@lusson-luo
Copy link
Contributor Author

@lusson-luo The feature is awesome, but the CI failed, please have a check.

@lusson-luo The feature is awesome, but the CI failed, please have a check.

@gqcn ok, i finish ci failed

@lusson-luo lusson-luo requested a review from gqcn May 16, 2023 02:25
@lusson-luo
Copy link
Contributor Author

@gqcn 大佬审核下 pr

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@gqcn boss review the pr

gqcn
gqcn requested changes May 22, 2023
View reviewed changes
database/gdb/gdb_model_select.go Outdated Show resolved Hide resolved
database/gdb/gdb_model_select.go Outdated Show resolved Hide resolved
contrib/drivers/sqlite/sqlite_model_test.go Show resolved Hide resolved
@gqcn
Copy link
Member

gqcn commented May 23, 2023

@lusson-luo I'm watching this PR.

@gqcn
Copy link
Member

gqcn commented May 29, 2023

@lusson-luo I'm watching this.

@lusson-luo
Copy link
Contributor Author

@gqcn 嗯嗯，上面的问题我都答复了，我认为 count(1) 可能比 count(filed) 更适合一点，理由也在上面

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@gqcn Well, I have answered all the above questions, I think count(1) may be more suitable than count(filed), the reason is also above

@gqcn gqcn changed the title feat: Add method ScanAndCount and AllAndCount feat: Add method ScanAndCount and AllAndCount for Model package gdb Jun 1, 2023
@gqcn gqcn changed the title feat: Add method ScanAndCount and AllAndCount for Model package gdb add method ScanAndCount and AllAndCount for Model package gdb Jun 1, 2023
@gqcn gqcn merged commit 00190ab into gogf:master Jun 1, 2023
@Porkepix Porkepix mentioned this pull request Jul 17, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houseme houseme houseme approved these changes

@gqcn gqcn gqcn approved these changes

@hailaz hailaz Awaiting requested review from hailaz

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

Successfully merging this pull request may close these issues.
5 participants
@lusson-luo @codecov-commenter @Issues-translate-bot @houseme @gqcn