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

Go: database source models for github.com/beego/beego/client/orm #18465

Merged

Conversation

egregius313
Copy link
Contributor

Adds models for the github.com/beego/beego/client/orm package.

Pull Request checklist

All query authors

Internal query authors only

- [ ] Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required).

@Copilot Copilot bot review requested due to automatic review settings January 10, 2025 02:56
@egregius313 egregius313 requested a review from a team as a code owner January 10, 2025 02:56

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Contributor

github-actions bot commented Jan 10, 2025

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

go

Generated file changes for go

  • Changes to framework-coverage-go.rst:
-    `beego <https://beego.me/>`_,"``github.com/astaxie/beego*``, ``github.com/beego/beego*``",63,63,213
+    `beego <https://beego.me/>`_,"``github.com/astaxie/beego*``, ``github.com/beego/beego*``",102,63,213
-    Totals,,420,943,1532
+    Totals,,459,943,1532
  • Changes to framework-coverage-go.csv:
- github.com/astaxie/beego,71,21,21,,,,34,,5,,,,,,30,2,,,,,,,,21,,21,
+ github.com/astaxie/beego,71,34,21,,,,34,,5,,,,,,30,2,,,,,13,,,21,,21,
- github.com/beego/beego,142,42,42,,,,68,,10,,,,,,60,4,,,,,,,,42,,42,
+ github.com/beego/beego,142,68,42,,,,68,,10,,,,,,60,4,,,,,26,,,42,,42,

Comment on lines +17 to +22
- ["group:beego-orm", "DQL", True, "Read", "", "", "Argument[0]", "database", "manual"]
- ["group:beego-orm", "DQL", True, "ReadWithCtx", "", "", "Argument[1]", "database", "manual"]
- ["group:beego-orm", "DQL", True, "ReadForUpdate", "", "", "Argument[0]", "database", "manual"]
- ["group:beego-orm", "DQL", True, "ReadForUpdateWithCtx", "", "", "Argument[1]", "database", "manual"]
- ["group:beego-orm", "DQL", True, "ReadOrCreate", "", "", "Argument[0]", "database", "manual"]
- ["group:beego-orm", "DQL", True, "ReadOrCreateWithCtx", "", "", "Argument[1]", "database", "manual"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed this in the last review, but in older versions (2/3 of the package paths), this type was called Ormer (here and here). These models should be duplicated for that type. I would just use the same package group, as the name isn't used in later versions.

Please also add tests for it. You'll need to make new stubs for one of the other package paths. Go makes it easy to import two versions of the same library in the same file - you just give one of them a different name (as in import "github.com/astaxie/beego/orm" oldorm) and use that when referring to them (oldorm.NewOrm() instead of orm.NewOrm()). So you can keep the tests in the same file. Or you can put them in a separate file, if you prefer.

Copy link
Contributor Author

@egregius313 egregius313 Jan 10, 2025

Choose a reason for hiding this comment

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

I added models for Ormer::Read, ReadForUpdate, and ReadOrCreate. The *Ctx variants appear to be new in v2.

go/ql/lib/change-notes/2025-01-09-beego-orm-models.md Outdated Show resolved Hide resolved
@egregius313 egregius313 requested a review from owen-mc January 10, 2025 13:24
owen-mc
owen-mc previously approved these changes Jan 10, 2025
@egregius313 egregius313 merged commit fd878a1 into github:main Jan 10, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants