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

Fixes 370: moves ovirt to internal directory #397

Merged
merged 1 commit into from
May 25, 2022

Conversation

engelmi
Copy link
Collaborator

@engelmi engelmi commented May 25, 2022

Please describe the change you are making

This PR fixes #370 and moves the ovirt directory to the internal directory.

Are you the owner of the code you are sending in, or do you have permission of the owner?

yes

The code will be published under the BSD 3 clause license. Have you read and understood this license?

yes

@engelmi engelmi force-pushed the 370-move-to-internal-dir branch from 1dfdc6d to 48a716d Compare May 25, 2022 11:21
@@ -1,8 +1,10 @@
//go:build tools
// +build tools
//go:build generate
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd move the go:generate to the import to have it all in one place. If this is not desired, I can revert it, of course.

Copy link

Choose a reason for hiding this comment

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

If you now run go generate does it actually generate? Because this build will exclude it from the normal build process.

Copy link
Collaborator Author

@engelmi engelmi May 25, 2022

Choose a reason for hiding this comment

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

According to https://pkg.go.dev/cmd/go/internal/generate:

The go generate tool also sets the build tag "generate" so that files may be examined by go generate but ignored during build.

It should generate it, yes. I tested it locally and it generated the doc when changing the tag to generate. (that is also the reason, why a change in the doc is in this PR)

Copy link

Choose a reason for hiding this comment

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

Maybe this would be something to submit to the SDK sample projects too?

@engelmi engelmi requested review from a user and eslutsky May 25, 2022 11:25
@engelmi engelmi force-pushed the 370-move-to-internal-dir branch from 48a716d to fc9d943 Compare May 25, 2022 12:45
Copy link
Collaborator

@eslutsky eslutsky left a comment

Choose a reason for hiding this comment

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

looks good

@engelmi engelmi merged commit 0342188 into oVirt:main May 25, 2022
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.

Move the code to the internal directory
2 participants