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

Refactor: transparent and extensible cuegen decl #6006

Merged
merged 1 commit into from
May 18, 2023

Conversation

iyear
Copy link
Contributor

@iyear iyear commented May 17, 2023

Description of your changes

🤖 Generated by Copilot at b3c4005

Summary

🧪🏗️♻️

This pull request refactors the CUE generator code to use a new Decl interface for different kinds of CUE declarations, such as structs, and adds a Struct type that implements the interface. This simplifies the code structure, makes it more extensible, and enables common fields and methods for CUE declarations. The pull request also updates the test cases and modifies the provider.go file to use the new interface.

We're the CUE generator crew, we make the code for you
We use the Decl interface to handle any case
We build the structs and fields with common methods and skills
We heave away and haul away and format them with grace

Walkthrough

  • Introduce a new interface Decl and a new type Struct that implement it in decl.go (link, link)
  • Change the return type of Generate and convertDecls methods from []cueast.Decl to []Decl in convert.go and generator.go (link, link)
  • Change the parameter type of Format and modifyDecls functions from []cueast.Decl to []Decl in generator.go and provider.go (link, link)
  • Refactor the logic of convertDecls to use a switch statement instead of an if statement for handling different kinds of underlying types in convert.go (link)
  • Modify the logic of Format to call the Build method of each Decl and append the result to the f.Decls slice in generator.go (link)
  • Modify the logic of modifyDecls to use a type assertion to check if each Decl is a Struct and use its fields to populate the mapping map in provider.go (link)
  • Modify the logic of modifyDecls to create and append a Struct instance instead of a cueast.Field instance to the decls slice in provider.go (link)
  • Remove the unused import of cueast in generator_test.go (link)
  • Change the parameter type and the argument type of the Format method calls in TestGeneratorFormat function from []cueast.Decl to []Decl in generator_test.go (link)
  • Add a unit test for the Struct type and its Build method in decl_test.go (link)

Part of #5364

We may support more CUE types in the future, so we need transparent and extensible cuegen decl.

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

No break changes and no modified unit tests.

Special notes for your reviewer

Signed-off-by: iyear <ljyngup@gmail.com>
@codecov
Copy link

codecov bot commented May 17, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.04 🎉

Comparison is base (da8588c) 60.81% compared to head (b3c4005) 60.86%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6006      +/-   ##
==========================================
+ Coverage   60.81%   60.86%   +0.04%     
==========================================
  Files         224      225       +1     
  Lines       31206    31345     +139     
==========================================
+ Hits        18979    19078      +99     
- Misses      10454    10492      +38     
- Partials     1773     1775       +2     
Flag Coverage Δ
core-unittests 56.01% <ø> (+0.03%) ⬆️
e2e-multicluster-test 24.93% <ø> (+0.11%) ⬆️
e2etests 25.28% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 21 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@Somefive Somefive left a comment

Choose a reason for hiding this comment

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

/cc @FogDong

@FogDong FogDong merged commit f3f2af8 into kubevela:master May 18, 2023
27 checks passed
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.

None yet

3 participants