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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 crd: Respect multiline comments at godocs #870

Conversation

qinqon
Copy link
Contributor

@qinqon qinqon commented Dec 20, 2023

Sometimes at type has examples about how to use it embedding something like a yaml on it, yamls should not be truncated and they are white space sensitive. This change keep the new lines and also remove the white space trimming only on /* comments.

Golang struct example with problematic comment.

// DnsState  DNS resolver state. Example partial yaml output of [NetworkState] with
// static DNS config:                                                           
// ```yaml                                                                      
// ---                                                                          
// dns-resolver:                                                                
//                                                                              
//  running:                                                                    
//     server:                                                                  
//     - 2001:db8:1::250                                                        
//     - 192.0.2.250                                                            
//     search:                                                                  
//     - example.org                                                            
//     - example.net                                                            
//  config:                                                                     
//     search:                                                                  
//     - example.org                                                            
//     - example.net                                                            
//     server:                                                                  
//     - 2001:db8:1::250                                                        
//     - 192.0.2.250                                                            
//     options:                                                                 
//     - trust-ad                                                               
//     - rotate                                                                 
//                                                                              
// ```                                                                          
// To purge all static DNS configuration:                                       
// ```yml                                                                       
// ---                                                                          
// dns-resolver:                                                                
//                                                                              
//  config: {}                                                                  
//                                                                              
// ```                                                                          
// +k8s:deepcopy-gen=true                                                       
type DnsState struct {                                                          
...
}

Part of the probem was introduced at #517

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 20, 2023
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 20, 2023
@qinqon qinqon changed the title crd: Respect multiline comments at godocs 馃悰 馃悰 crd: Respect multiline comments at godocs Dec 20, 2023
@qinqon qinqon force-pushed the crd-description-respect-multiline-comments-at-godoc branch from 68cbc38 to a8aa71c Compare December 20, 2023 10:36
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 20, 2023
@qinqon
Copy link
Contributor Author

qinqon commented Dec 20, 2023

/cc @varshaprasad96

Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

This change looks good to me, but it would break a lot of existing docs. @qinqon Some of the integration tests are failing due to formating changes, could you please fix them? (https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-tools/870/pull-controller-tools-test-master/1737421873815228416#)

cc: @alvaroaleman any thoughts on this? Making this change optional may be an overkill. Having this introduced in a new release is still fine?

@qinqon
Copy link
Contributor Author

qinqon commented Dec 20, 2023

This change looks good to me, but it would break a lot of existing docs. @qinqon Some of the integration tests are failing due to formating changes, could you please fix them? (https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_controller-tools/870/pull-controller-tools-test-master/1737421873815228416#)

cc: @alvaroaleman any thoughts on this? Making this change optional may be an overkill. Having this introduced in a new release is still fine?

Ok, let me fix stuff.

@qinqon qinqon force-pushed the crd-description-respect-multiline-comments-at-godoc branch from a8aa71c to 3dee960 Compare December 20, 2023 14:12
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 20, 2023
@qinqon qinqon force-pushed the crd-description-respect-multiline-comments-at-godoc branch 2 times, most recently from 43f22dd to e6e1a30 Compare December 20, 2023 14:34
Sometimes at type has examples about how to use it embedding something
like a yaml on it, yamls should not be truncated and they are white
space sensitive. This change keep the new lines and also remove the
white space trimming only on /* comments.

Signed-off-by: Enrique Llorente <ellorent@redhat.com>
@qinqon qinqon force-pushed the crd-description-respect-multiline-comments-at-godoc branch from e6e1a30 to 7488021 Compare December 20, 2023 15:00
@qinqon qinqon changed the title 馃悰 crd: Respect multiline comments at godocs 馃悰 HOSTEDCP-1364, crd: Respect multiline comments at godocs Dec 21, 2023
@qinqon qinqon changed the title 馃悰 HOSTEDCP-1364, crd: Respect multiline comments at godocs 馃悰 crd: Respect multiline comments at godocs Dec 21, 2023
@qinqon
Copy link
Contributor Author

qinqon commented Dec 21, 2023

@varshaprasad96 @alvaroaleman test fixed, I have regenerate all the testing CRDs impacted so you can check how "description" field looks now.

Copy link
Member

@varshaprasad96 varshaprasad96 left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 2, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, qinqon, varshaprasad96

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 2, 2024
@k8s-ci-robot k8s-ci-robot merged commit 419500a into kubernetes-sigs:master Jan 2, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants