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

lxddoc: Renamed to lxd-metadata #12171

Merged
merged 3 commits into from
Aug 22, 2023

Conversation

gabrielmougard
Copy link
Contributor

@gabrielmougard gabrielmougard commented Aug 22, 2023

Installing lxddoc like previously in the makefile (go install ... /lxd/config/generate) was creating a binary named generate which was in conflict with an other lxd tool (the one at lxd/db/generate).

@github-actions github-actions bot added the Documentation Documentation needs updating label Aug 22, 2023
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

why is lxddoc under config dir?

@gabrielmougard
Copy link
Contributor Author

For historical reasons I guess... When the tool was first introduced, as it had to do with documenting the config keys of the projects, it has been put there. But I'm not against putting it in lxd/lxddoc

@tomponline
Copy link
Member

For historical reasons I guess... When the tool was first introduced, as it had to do with documenting the config keys of the projects, it has been put there. But I'm not against putting it in lxd/lxddoc

Yes I suppose this should be called lxd/lxd-metadata now right? Given that @ru-fu says the output isn't to be considered documentation anyway.

@tomponline
Copy link
Member

was creating a binary named generate which was in conflict with an other lxd tool (the one at lxd/db/generate).

BTW I would like to understand how you were getting a generate command being built from lxd/db/generate as the make file builds it like this:

cd lxd/db/generate && go build -o $(GOPATH)/bin/lxd-generate -tags "$(TAG_SQLITE3)" $(DEBUG) && cd -

@gabrielmougard
Copy link
Contributor Author

gabrielmougard commented Aug 22, 2023

was creating a binary named generate which was in conflict with an other lxd tool (the one at lxd/db/generate).

BTW I would like to understand how you were getting a generate command being built from lxd/db/generate as the make file builds it like this:

cd lxd/db/generate && go build -o $(GOPATH)/bin/lxd-generate -tags "$(TAG_SQLITE3)" $(DEBUG) && cd -

I don't understand either. Tried to remove the binary iin my go/bin folder and re-executed make build and my latest master and it still appears back in my go/bin..

This is linked to this line CC="$(CC)" CGO_LDFLAGS_ALLOW="$(CGO_LDFLAGS_ALLOW)" go install -v -tags "$(TAG_SQLITE3)" $(DEBUG) ./...

@gabrielmougard
Copy link
Contributor Author

So the generate binary we have (generated at make build) is the same as lxd-generate (generated at make update-schema)

@tomponline
Copy link
Member

was creating a binary named generate which was in conflict with an other lxd tool (the one at lxd/db/generate).

BTW I would like to understand how you were getting a generate command being built from lxd/db/generate as the make file builds it like this:

cd lxd/db/generate && go build -o $(GOPATH)/bin/lxd-generate -tags "$(TAG_SQLITE3)" $(DEBUG) && cd -

I don't understand either. Tried to remove the binary iin my go/bin folder and re-executed make build and my latest master and it still appears back in my go/bin..

This is linked to this line CC="$(CC)" CGO_LDFLAGS_ALLOW="$(CGO_LDFLAGS_ALLOW)" go install -v -tags "$(TAG_SQLITE3)" $(DEBUG) ./...

ah yes that will be why then

@tomponline
Copy link
Member

we should probably rename db/generate to db/lxd-generate too, but separately later.

@gabrielmougard gabrielmougard force-pushed the fix/rename-lxddoc-folder branch from 4c7f06e to 7c23e9b Compare August 22, 2023 10:36
@gabrielmougard
Copy link
Contributor Author

@tomponline now it should be clearer

@tomponline tomponline changed the title rename lxddoc source folder lxddoc: Renamed to lxd-metadata Aug 22, 2023
@ru-fu
Copy link
Contributor

ru-fu commented Aug 22, 2023

why is lxddoc under config dir?

This is why: #11652 (comment)

I actually prefer "config" to "metadata", because the tool generates different output for config options - the metadata for consumption by the UI and the documentation for consumption by Sphinx.
But "lxd-metadata" isn't worse than "lxddoc" of course, covering one of the two purposes. So I can totally live with it.

@tomponline
Copy link
Member

why is lxddoc under config dir?

This is why: #11652 (comment)

I actually prefer "config" to "metadata", because the tool generates different output for config options - the metadata for consumption by the UI and the documentation for consumption by Sphinx. But "lxd-metadata" isn't worse than "lxddoc" of course, covering one of the two purposes. So I can totally live with it.

Im thinking we may use it for items other than config metadata in the future too.

@tomponline tomponline merged commit 8278714 into canonical:main Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants