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

Split helm chart into another repo #4125

Merged
merged 2 commits into from
Dec 3, 2021
Merged

Split helm chart into another repo #4125

merged 2 commits into from
Dec 3, 2021

Conversation

JaredTan95
Copy link
Member

@JaredTan95 JaredTan95 commented Dec 2, 2021

What's the purpose of this PR

Split helm chart into another repo: https://github.com/apolloconfig/apollo-helm-chart
@nobodyiam We need to update the domain point to new helm chart? Or Merge this after new helm chart works.

Which issue(s) this PR fixes:

Fixes ##3959

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Update the CHANGES log.

@JaredTan95 JaredTan95 added this to the 2.0.0 milestone Dec 2, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2021

Codecov Report

Merging #4125 (54c3642) into master (75f9950) will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #4125      +/-   ##
============================================
- Coverage     52.62%   52.52%   -0.10%     
+ Complexity     2616     2609       -7     
============================================
  Files           484      484              
  Lines         15186    15186              
  Branches       1572     1572              
============================================
- Hits           7991     7977      -14     
- Misses         6641     6656      +15     
+ Partials        554      553       -1     
Impacted Files Coverage Δ
...work/apollo/biz/message/DatabaseMessageSender.java 50.00% <0.00%> (-14.59%) ⬇️
...framework/apollo/openapi/entity/ConsumerAudit.java 42.42% <0.00%> (-6.07%) ⬇️
...rk/apollo/spring/property/SpringValueRegistry.java 83.78% <0.00%> (-5.41%) ⬇️
...nfigservice/service/AccessKeyServiceWithCache.java 82.65% <0.00%> (-2.05%) ⬇️
.../framework/apollo/spring/property/SpringValue.java 87.71% <0.00%> (-1.76%) ⬇️
...mework/apollo/openapi/service/ConsumerService.java 53.38% <0.00%> (-1.70%) ⬇️
.../apollo/internals/RemoteConfigLongPollService.java 78.31% <0.00%> (+1.20%) ⬆️

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 75f9950...54c3642. Read the comment docs.

@nobodyiam
Copy link
Member

@JaredTan95

The url https://www.apolloconfig.com/charts currently points to the charts directory, I think we also need to copy this charts folder to apollo-helm-chart.
As for the domain for apollo-helm-chart, how about charts.apolloconfig.com?
BTW, to keep backward compatibility, I think the charts directory should remain for a while so that users who haven't updated the charts repo could still fetch the current versions.

@nobodyiam
Copy link
Member

I have copied the charts directory to docs directory of apollo-helm-chart. And now the new helm repo url is https://charts.apolloconfig.com/.
So we could now merge this pr and I will submit another pr to update the chart repo URL later.

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit c926429 into master Dec 3, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2021
@nobodyiam nobodyiam deleted the split_helm_chart branch December 3, 2021 01:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants