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: capabilities and workload in appfile parsing #6250

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

chivalryq
Copy link
Member

@chivalryq chivalryq commented Aug 9, 2023

Signed-off-by: Qiaozp [email protected]

Description of your changes

Main changes are:

  1. Tidy appfile parse process, remove unused properties, rename workload->component
  2. Tidy Capability properties

🤖 Generated by Copilot at d4d2734

Summary

🚚🛠️📝

Refactor the appfile package and related controllers to use the Component and ParsedComponents types instead of the Workload and Workloads types, and remove unused or deprecated code related to helm charts, CRDs, and labels. This improves the consistency, readability, and maintainability of the code and aligns with the application model.

Oh we're the crew of the appfile ship, and we sail the code sea
We've changed the Workload to Component, and the ParsedComponents we need
So heave away, heave away, heave away on the count of three
We've simplified the types and names, and improved the consistency

Walkthrough

  • Remove unused types and fields from the types and Capability packages (link, link, link)
  • Update the import path and setup function of the oamv1beta1 package in the server package (link, link)
  • Update the expected output of the dry-run and diff commands in the plugin test, from Component to ParsedComponents (link, link, link, link, link, link, link)
  • Update the title of the custom component header in the mods package, from Built-in Component Type to Built-in ParsedComponents Type (link)
  • Rename the Workload type to Component and the Workloads field to ParsedComponents in the appfile package, and remove unused fields and parameters (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Update the calls and signatures of the newAppFile, parseComponents, parseWorkflowSteps, fetchAndSetWorkflowStepDefinition, makeComponent, convertTemplate2Component, ParseComponentFromRevision, ParseComponentFromRevisionAndClient, and parseWorkflowStepsForLegacyRevision functions in the appfile package (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Update the tests in the appfile package to use the Component and ParsedComponents types (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Update the ValidateCUESchematicAppfile and newValidationProcessContext functions in the appfile package to use the Component type (link, link, link)
  • Update the application controller to use the ParsedPolicies and Component types (link, link)
  • Update the apply and generator functions in the application controller to use the Component type and change the parameter and return types accordingly (link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Update the revision functions in the application controller to use the ParsedComponents and ParsedPolicies types (link, link, link)
  • Remove unused constants from the oam package (link, link, link, link, link)
  • Update the WorkloadRenderer type alias in the oam package to use the Component type (link)
  • Update the fakeWorkloadRenderer function in the terraform test to use the Component type (link)

Fixes #

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

Special notes for your reviewer

Signed-off-by: Qiaozp <[email protected]>
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch coverage: 79.42% and project coverage change: +0.13% 🎉

Comparison is base (72bb079) 66.09% compared to head (9b66686) 66.22%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6250      +/-   ##
==========================================
+ Coverage   66.09%   66.22%   +0.13%     
==========================================
  Files         183      183              
  Lines       24064    24076      +12     
==========================================
+ Hits        15905    15945      +40     
+ Misses       6583     6556      -27     
+ Partials     1576     1575       -1     
Flag Coverage Δ
core-unittests 55.40% <69.95%> (+0.07%) ⬆️
e2e-multicluster-test 30.96% <61.72%> (+0.10%) ⬆️
e2etests 32.63% <61.31%> (-0.05%) ⬇️

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

Files Changed Coverage Δ
cmd/core/app/server.go 62.12% <0.00%> (ø)
pkg/workflow/providers/oam/apply.go 57.96% <ø> (ø)
pkg/appfile/validate.go 75.00% <66.66%> (ø)
...ntroller/core.oam.dev/v1beta1/application/apply.go 85.95% <69.56%> (+2.05%) ⬆️
pkg/appfile/appfile.go 67.51% <79.22%> (ø)
pkg/appfile/parser.go 72.79% <80.16%> (-0.54%) ⬇️
...ller/core.oam.dev/v1beta1/application/generator.go 83.95% <85.71%> (ø)
....dev/v1beta1/application/application_controller.go 83.96% <100.00%> (ø)
...ler/core.oam.dev/v1beta1/application/dispatcher.go 92.00% <100.00%> (+2.40%) ⬆️
...oller/core.oam.dev/v1beta1/application/revision.go 70.87% <100.00%> (ø)

... and 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chivalryq
Copy link
Member Author

ping @StevenLeiZhang @jefree-cat

@chivalryq chivalryq merged commit bab5bb2 into kubevela:master Aug 10, 2023
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.

2 participants