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

Update samples for "vela show xxx --web" #1616

Merged
merged 8 commits into from
May 9, 2021

Conversation

zzxwill
Copy link
Collaborator

@zzxwill zzxwill commented May 7, 2021

Also stop generating components/traits docs under
docs/en/end-user and remove the command from Makefile

Fix #1602

@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #1616 (cce3e84) into master (1660930) will increase coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1616      +/-   ##
==========================================
+ Coverage   62.00%   62.15%   +0.15%     
==========================================
  Files         120      120              
  Lines       11899    11900       +1     
==========================================
+ Hits         7378     7397      +19     
+ Misses       3761     3734      -27     
- Partials      760      769       +9     
Flag Coverage Δ
e2etests 43.72% <ø> (+0.03%) ⬆️
unittests 58.02% <ø> (+0.10%) ⬆️

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

Impacted Files Coverage Δ
pkg/dsl/model/instance.go 75.67% <0.00%> (-5.41%) ⬇️
pkg/appfile/appfile.go 76.09% <0.00%> (-1.60%) ⬇️
...dev/v1alpha2/application/application_controller.go 79.13% <0.00%> (-1.44%) ⬇️
...ev/v1alpha2/core/scopes/healthscope/healthscope.go 90.40% <0.00%> (-1.02%) ⬇️
pkg/dsl/definition/template.go 48.38% <0.00%> (-0.93%) ⬇️
...oam.dev/v1alpha2/applicationconfiguration/apply.go 67.43% <0.00%> (-0.92%) ⬇️
...troller/core.oam.dev/v1alpha2/application/apply.go 72.86% <0.00%> (-0.49%) ⬇️
...a2/applicationrollout/applicationrollout_helper.go 65.44% <0.00%> (ø)
...pplicationrollout/applicationrollout_controller.go 81.18% <0.00%> (+1.07%) ⬆️
...g/controller/core.oam.dev/v1alpha2/core/revison.go 72.97% <0.00%> (+1.80%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1660930...cce3e84. Read the comment docs.

specification := fmt.Sprintf("\n\n## Specification\n\n%s\n\n%s", specificationIntro, specificationContent)
title := fmt.Sprintf(`---
title: %s
---`, capNameInTitle)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we change the style?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To make it consistent with the reference doc in kubevela.io https://kubevela.io/docs/developers/references/component-types/webservice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Will it still work in vela show --web?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change should be reverted, the new title style is not good if I didn't upgrade the Markdown hosting server. Thanks.
image

@wonderflow
Copy link
Collaborator

please create a new issue about "generate the sample automatically (low priority)" if you want to close #1602

zzxwill and others added 3 commits May 7, 2021 23:25
Also stop generating components/traits docs under
docs/en/end-user and remove the command from Makefile

Fix kubevela#1602
Co-authored-by: Jianbo Sun <wonderflow.sun@gmail.com>
@zzxwill zzxwill changed the title Update samples for "vela show xxx --web" [WIP] Update samples for "vela show xxx --web" May 8, 2021
- Find the capability in "vela-system" if notfound in the current ns
- Don't depend on local capabilities
@zzxwill zzxwill changed the title [WIP] Update samples for "vela show xxx --web" Update samples for "vela show xxx --web" May 8, 2021
@zzxwill
Copy link
Collaborator Author

zzxwill commented May 8, 2021

Fixed the issue of NotFound capability if the capability, like Terraform alibaba-oss, is not in "vela-system"
image

@zzxwill
Copy link
Collaborator Author

zzxwill commented May 8, 2021

In the first time, the coverage was dropped by 0.09%.
After adding one ut for a newly added function , it was dropped by 3.76%.
Now I added two more, watching the changing.

@zzxwill
Copy link
Collaborator Author

zzxwill commented May 8, 2021

In the first time, the coverage was dropped by 0.09%.
After adding one ut for a newly added function , it was dropped by 3.76%.
Now I added two more, watching the changing.

Now it dropped more by 3.86%.

@@ -57,16 +57,46 @@ func GetCapabilitiesFromCluster(ctx context.Context, namespace string, c common.
return workloads, nil
}

// GetNamespacedCapabilitiesFromCluster will get capability from K8s cluster in the specified namespace and default namespace
// If the definition could be found from `namespace`, try to find in namespace `types.DefaultKubeVelaNS`
Copy link
Collaborator

Choose a reason for hiding this comment

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

good job!

"github.com/google/go-cmp/cmp"
"io/ioutil"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

please format it.

// go inner packages

// third party packages

// project  pakcages

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Thanks.

Copy link
Collaborator

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

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

LGTM after nit

@wonderflow wonderflow merged commit f137a9a into kubevela:master May 9, 2021
@zzxwill zzxwill deleted the vela-show-web branch May 10, 2021 02:24
yangsoon pushed a commit to yangsoon/kubevela that referenced this pull request May 13, 2021
* Update samples for "vela show xxx --web"

Also stop generating components/traits docs under
docs/en/end-user and remove the command from Makefile

Fix kubevela#1602

* fix doc build issue

* Update references/plugins/references.go

Co-authored-by: Jianbo Sun <wonderflow.sun@gmail.com>

* Enhance --web feature for "vela show"

- Find the capability in "vela-system" if notfound in the current ns
- Don't depend on local capabilities

* add ut for GetCapabilityByName in references/plugins/cluster.go

* add ut for GetNamespacedCapabilitiesFromCluster in references/plugins/cluster.go

* add ut for GenerateTerraformCapabilityProperties in references/plugins/references.go

* fix import issue

Co-authored-by: Jianbo Sun <wonderflow.sun@gmail.com>
yangsoon pushed a commit to yangsoon/kubevela that referenced this pull request May 17, 2021
* Update samples for "vela show xxx --web"

Also stop generating components/traits docs under
docs/en/end-user and remove the command from Makefile

Fix kubevela#1602

* fix doc build issue

* Update references/plugins/references.go

Co-authored-by: Jianbo Sun <wonderflow.sun@gmail.com>

* Enhance --web feature for "vela show"

- Find the capability in "vela-system" if notfound in the current ns
- Don't depend on local capabilities

* add ut for GetCapabilityByName in references/plugins/cluster.go

* add ut for GetNamespacedCapabilitiesFromCluster in references/plugins/cluster.go

* add ut for GenerateTerraformCapabilityProperties in references/plugins/references.go

* fix import issue

Co-authored-by: Jianbo Sun <wonderflow.sun@gmail.com>
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.

The doc generation is out-of-date
2 participants