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

Deprecate Charts as they have been moved #2720

Merged
merged 2 commits into from
Dec 12, 2020
Merged

Deprecate Charts as they have been moved #2720

merged 2 commits into from
Dec 12, 2020

Conversation

torstenwalter
Copy link
Contributor

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #2593

Special notes for your reviewer:

This should only be merged once grafana/helm-charts#39 is merged and we confirmed that everything is working.

Checklist

  • Documentation added
  • Tests updated

@CLAassistant
Copy link

CLAassistant commented Oct 3, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2020 •
edited
Loading

Codecov Report

Merging #2720 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2720   +/-   ##
=======================================
  Coverage   61.40%   61.40%           
=======================================
  Files         173      173           
  Lines       13446    13446           
=======================================
  Hits         8257     8257           
  Misses       4431     4431           
  Partials      758      758

@torstenwalter torstenwalter mentioned this pull request Oct 3, 2020
Closed
scottrigby
scottrigby reviewed Oct 5, 2020
View reviewed changes
Copy link
Contributor

@scottrigby scottrigby left a comment

Choose a reason for hiding this comment

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

@torstenwalter This looks great. Since these charts will not become "un-deprecated" here, do you want to make a follow-up PR to remove the source code from this repo per https://helm.sh/docs/topics/charts/#deprecating-charts?

If you prefer to leave the deprecated source code here, would you instead also add a deprecation note to the templates/NOTES.txt file in each of these charts?

@owen-d
Copy link
Member

owen-d commented Oct 7, 2020

It'd be nice to remove the source eventually as @scottrigby suggested. All in all, looks great!

@stale
Copy link

stale bot commented Nov 7, 2020

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Nov 7, 2020
@owen-d owen-d removed the stale A stale issue or PR that will automatically be closed. label Nov 7, 2020
@stale
Copy link

stale bot commented Dec 8, 2020

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Dec 8, 2020
@torstenwalter
Copy link
Contributor Author

@owen-d @scottrigby does it make sense to deprecate the charts here at all or should we simple remove the source code here and leave a note where to find it instead?

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Dec 10, 2020
Deprecate Charts as they have been moved
47ae69e 
Signed-off-by: Torsten Walter <torsten.walter@syncier.com>
@torstenwalter torstenwalter marked this pull request as ready for review December 10, 2020 01:42
@scottrigby
Copy link
Contributor

We want to at least create a release to the repo with a deprecated version, so that existing users will have a way to know where to migrate to for future versions.

@torstenwalter
Copy link
Contributor Author

Makes sense. This PR is already updated to do that.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 11, 2020
slim-bean
slim-bean approved these changes Dec 12, 2020
View reviewed changes
Copy link
Collaborator

@slim-bean slim-bean left a comment

Choose a reason for hiding this comment

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

LGTM

@slim-bean slim-bean merged commit 6c8ad52 into grafana:master Dec 12, 2020
@ilpianista ilpianista mentioned this pull request Dec 16, 2020
Merged
mraboosk pushed a commit to mraboosk/loki that referenced this pull request Oct 7, 2024
@torstenwalter @slim-bean
Deprecate Charts as they have been moved (grafana#2720)
2a86d56 
* Deprecate Charts as they have been moved

Signed-off-by: Torsten Walter <torsten.walter@syncier.com>

* update docs and links to new chart location

Co-authored-by: Edward Welch <edward.welch@grafana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottrigby scottrigby scottrigby left review comments

@slim-bean slim-bean slim-bean approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Central helm charts repository for grafana?
6 participants
@torstenwalter @CLAassistant @codecov-commenter @owen-d @scottrigby @slim-bean