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

docs: update go structs with updated format for docgen #8198

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andrewrynhard
Copy link
Member

No description provided.

Copy link
Member

@smira smira left a comment

Choose a reason for hiding this comment

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

this would break machine config comments, JSON schema generation, etc.

it doesn't cover new machine config documents either

@andrewrynhard
Copy link
Member Author

andrewrynhard commented Jan 24, 2024

this would break machine config comments, JSON schema generation, etc.

it doesn't cover new machine config documents either

@smira
Copy link
Member

smira commented Jan 24, 2024

I think this need to be planned properly with exact ideas on what is going to be dropped, moved, removed, etc.

Can we do it with more transitional changes and less breaking changes?

How it's going to look in the end and so on. So far it won't pass the CI even.

@andrewrynhard
Copy link
Member Author

I think this need to be planned properly with exact ideas on what is going to be dropped, moved, removed, etc.

Can we do it with more transitional changes and less breaking changes?

How it's going to look in the end and so on. So far it won't pass the CI even.

This is a draft so I don't expect it to pass just yet. I posted in our private Slack I would like to discuss this week. This is just here in the form it is now to help illustrate the changes so we can have a more meaningful discussion.

@andrewrynhard andrewrynhard force-pushed the docs branch 2 times, most recently from eb16550 to 0b24336 Compare April 28, 2024 00:43
@andrewrynhard
Copy link
Member Author

Picking this back up finally. Not ready for review yet but I want to use CI for testing. I plan on basing other PRs off of this one and going through a series of PRs that we can get aligned and push through.

@andrewrynhard andrewrynhard marked this pull request as ready for review April 28, 2024 00:47
@andrewrynhard andrewrynhard force-pushed the docs branch 2 times, most recently from a23d5cc to 24b7ef6 Compare April 28, 2024 01:20
@andrewrynhard
Copy link
Member Author

Not ready for review. Using CI to test.

@andrewrynhard andrewrynhard force-pushed the docs branch 17 times, most recently from 39a6532 to 82b5b14 Compare April 28, 2024 17:17
@andrewrynhard andrewrynhard force-pushed the docs branch 3 times, most recently from 0d279b8 to 57e68c8 Compare May 4, 2024 00:34
Copy link

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Stale label Feb 14, 2025
Updates Go types for compatibility with an external implementation
of docgen.

Signed-off-by: Andrew Rynhard <[email protected]>
@@ -195,7 +195,7 @@ func (in *Input) init() ([]config.Document, error) {
cluster.AllowSchedulingOnControlPlanes = pointer.To(in.Options.AllowSchedulingOnControlPlanes)
} else {
// backwards compatibility for Talos versions older than 1.2
cluster.AllowSchedulingOnMasters = pointer.To(in.Options.AllowSchedulingOnControlPlanes) //nolint:staticcheck
cluster.AllowSchedulingOnMasters = pointer.To(in.Options.AllowSchedulingOnControlPlanes)
Copy link
Member

Choose a reason for hiding this comment

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

this is a regression - the field was deprecated, and should stay so to avoid accidental use

// Specifies the machine's role within the cluster. The roles can be either "controlplane" or "worker".
// The "controlplane" role hosts etcd and the Kubernetes control plane components such as API Server, Controller Manager, Scheduler.
// The "worker" role is available for scheduling workloads.
MachineType string `yaml:"type" docgen:"{'values':['controlplane','worker'],'in':'1.5'}"`
Copy link
Member

Choose a reason for hiding this comment

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

not sure what in means, but this field was there forever, not since Talos 1.5

@andrewrynhard
Copy link
Member Author

@smira This PR is a draft as I work out some things with the framework. You can ignore this PR for now. I will mark this as ready for review when the time comes. Thanks.

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