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.

Sign up for GitHub

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

Jump to bottom

Don't hardcode properties in newConvertToARMFunctionBuilder #3549

Closed
matthchr opened this issue Nov 13, 2023 · 0 comments · Fixed by #3685
Closed

Don't hardcode properties in newConvertToARMFunctionBuilder #3549

matthchr opened this issue Nov 13, 2023 · 0 comments · Fixed by #3685
Assignees
@matthchr
Labels
technical-debt 📈
Milestone
v2.6.0

Comments

@matthchr
Copy link
Member

Describe the current behavior
v2/tools/generator/internal/armconversion/convert_to_arm_function_builder.go has the following code snippet

	var emptyCollectionProperties set.Set[string]
	if forceEmptyCollections {
		// TODO: These should not be hardcoded here, instead should be in the azure-arm.yaml config
		emptyCollectionProperties = set.Make(
			"Tags",
			"NodeLabels",
			"NodeTaints")
	}

Describe the improvement
The properties should be specified in the configuration file, not in code.
@matthchr matthchr added this to the v2.5.0 milestone Nov 13, 2023
@matthchr matthchr self-assigned this Nov 13, 2023
@theunrepentantgeek theunrepentantgeek modified the milestones: v2.6.0, v2.5.0 Nov 14, 2023
@theunrepentantgeek theunrepentantgeek modified the milestones: v2.5.0, v2.6.0, v2.7.0 Dec 11, 2023
@matthchr matthchr modified the milestones: v2.7.0, v2.6.0 Jan 2, 2024
matthchr added a commit to matthchr/azure-service-operator that referenced this issue Jan 5, 2024
@matthchr
Stop hardcoding properties in newConvertToARMFunctionBuilder
b6781a2 
This fixes Azure#3549.

This uses a property tag for a few reasons:
  1. Some types are flattened and other approaches are easily
     lost when flattening occurs. Putting the tag onto the property
     preserves it even through flattening.
  2. In many ways this behavior is like an augmented `json:omitempty`,
     so it (IMO) makes sense to have a tag just like for JSON. It makes
     it clearer when looking at the Go object that the serialization behavior
     of these fields is special.

This also fixes an issue with the configuration where hierarchical
configurations didn't propagate the checks for usage.
@matthchr matthchr mentioned this issue Jan 5, 2024
Merged
3 tasks
matthchr added a commit to matthchr/azure-service-operator that referenced this issue Jan 5, 2024
@matthchr
Stop hardcoding properties in newConvertToARMFunctionBuilder
bb179b4 
This fixes Azure#3549.

This uses a property tag for a few reasons:
  1. Some types are flattened and other approaches are easily
     lost when flattening occurs. Putting the tag onto the property
     preserves it even through flattening.
  2. In many ways this behavior is like an augmented `json:omitempty`,
     so it (IMO) makes sense to have a tag just like for JSON. It makes
     it clearer when looking at the Go object that the serialization behavior
     of these fields is special.

This also fixes an issue with the configuration where hierarchical
configurations didn't propagate the checks for usage.
matthchr added a commit to matthchr/azure-service-operator that referenced this issue Jan 11, 2024
@matthchr
Stop hardcoding properties in newConvertToARMFunctionBuilder
7ce0170 
This fixes Azure#3549.

This uses a property tag for a few reasons:
  1. Some types are flattened and other approaches are easily
     lost when flattening occurs. Putting the tag onto the property
     preserves it even through flattening.
  2. In many ways this behavior is like an augmented `json:omitempty`,
     so it (IMO) makes sense to have a tag just like for JSON. It makes
     it clearer when looking at the Go object that the serialization behavior
     of these fields is special.

This also fixes an issue with the configuration where hierarchical
configurations didn't propagate the checks for usage.
github-merge-queue bot pushed a commit that referenced this issue Jan 11, 2024
@matthchr
Stop hardcoding properties in newConvertToARMFunctionBuilder (#3685)
43a5635 
This fixes #3549.

This uses a property tag for a few reasons:
  1. Some types are flattened and other approaches are easily
     lost when flattening occurs. Putting the tag onto the property
     preserves it even through flattening.
  2. In many ways this behavior is like an augmented `json:omitempty`,
     so it (IMO) makes sense to have a tag just like for JSON. It makes
     it clearer when looking at the Go object that the serialization behavior
     of these fields is special.

This also fixes an issue with the configuration where hierarchical
configurations didn't propagate the checks for usage.
@github-project-automation github-project-automation bot moved this from Backlog to Recently Completed in Azure Service Operator Roadmap Jan 11, 2024
@matthchr matthchr moved this from Recently Completed to Ready for Release in Azure Service Operator Roadmap Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthchr matthchr

Labels
technical-debt 📈
Projects
Azure Service Operator Roadmap
Archived in project
Milestone
v2.6.0
Development

Successfully merging a pull request may close this issue.

Stop hardcoding properties in newConvertToARMFunctionBuilder matthchr/azure-service-operator
2 participants
@theunrepentantgeek @matthchr