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

[Feat]: Add a demo of golang gorm #56

Merged
merged 6 commits into from
May 29, 2024
Merged

[Feat]: Add a demo of golang gorm #56

merged 6 commits into from
May 29, 2024

Conversation

yyt030
Copy link
Contributor

@yyt030 yyt030 commented May 25, 2024

Summary

Solution Description

@CLAassistant
Copy link

CLAassistant commented May 25, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@whhe whhe left a comment

Choose a reason for hiding this comment

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

@yyt030 Thanks for your contribution! I left some comments. And you also need to add the gorm module in CI to ensure that the sample code is executable.

For the mydata directory in applications, it looks good overall, but I think it should be added in another PR instead of being included in the current PR. Can you split it out from this PR?

golang/gorm/README.md Outdated Show resolved Hide resolved
golang/gorm/README.md Outdated Show resolved Hide resolved
@yyt030
Copy link
Contributor Author

yyt030 commented May 29, 2024

@yyt030 Thanks for your contribution! I left some comments. And you also need to add the gorm module in CI to ensure that the sample code is executable.

For the mydata directory in applications, it looks good overall, but I think it should be added in another PR instead of being included in the current PR. Can you split it out from this PR?

I opened a new PR, please check. #58

@yyt030 yyt030 requested a review from whhe May 29, 2024 12:46
@whhe
Copy link
Member

whhe commented May 29, 2024

Thanks for the update! The CI configuration for the newly added gorm module is still missing, could you please add it to .github/workflows/ci.yml? You can refer to the repository's readme file to learn how to set up a basic CI workflow.

@yyt030
Copy link
Contributor Author

yyt030 commented May 29, 2024

Thanks for the update! The CI configuration for the newly added gorm module is still missing, could you please add it to .github/workflows/ci.yml? You can refer to the repository's readme file to learn how to set up a basic CI workflow.

done

@yyt030
Copy link
Contributor Author

yyt030 commented May 29, 2024

@whhe pls rerun ci?

@whhe
Copy link
Member

whhe commented May 29, 2024

Seems the command line arguments are not passed correctly.

2024/05/29 14:44:32 /home/runner/work/ob-samples/ob-samples/golang/gorm/example.go:47
[error] failed to initialize database, got error dial tcp [12](https://github.com/oceanbase/ob-samples/actions/runs/9287971069/job/25559066979?pr=56#step:7:13)7.0.0.1:3306: connect: connection refused
panic: failed to connect database

goroutine 1 [running]:
main.main()
	/home/runner/work/ob-samples/ob-samples/golang/gorm/example.go:49 +0x5a5
exit status 2

@yyt030
Copy link
Contributor Author

yyt030 commented May 29, 2024

Seems the command line arguments are not passed correctly.

2024/05/29 14:44:32 /home/runner/work/ob-samples/ob-samples/golang/gorm/example.go:47
[error] failed to initialize database, got error dial tcp [12](https://github.com/oceanbase/ob-samples/actions/runs/9287971069/job/25559066979?pr=56#step:7:13)7.0.0.1:3306: connect: connection refused
panic: failed to connect database

goroutine 1 [running]:
main.main()
	/home/runner/work/ob-samples/ob-samples/golang/gorm/example.go:49 +0x5a5
exit status 2

I install one archlinux with mariadb server in virtualbox, and debug that then deal it , connect info must include Net, otherwise use 3306 default port.

it's difficult that without test OB env.

@whhe try it again.

Copy link
Member

@whhe whhe left a comment

Choose a reason for hiding this comment

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

Great work! Merging.

@whhe whhe merged commit 02c4b66 into oceanbase:master May 29, 2024
14 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.

3 participants