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

Fix: defaultCliVersion in values.yaml is not up to date with latest cli version #300

Closed

Conversation

TonyOuyangGit
Copy link
Contributor

This PR is a quick fix to datahub/values.yaml to update defaultCliVersion to use v0.10.2 to align with the latest changes, otherwise, the ingestion run will still use v0.10.0 as the default cli version.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

Copy link
Contributor

@jinlintt jinlintt left a comment

Choose a reason for hiding this comment

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

Thank you @TonyOuyangGit

@justmike1
Copy link
Contributor

justmike1 commented Apr 18, 2023

why not just deleting this as a value and using {{ trimPrefix "v" .Values.global.datahub.version }} Instead? @jinlintt

@jinlintt
Copy link
Contributor

why not just deleting this as a value and using {{ trimPrefix "v" .Values.global.datahub.version }} Instead? @jinlintt

@justmike1
For backward compatibility, we shouldn't delete any variables. Otherwise, for people who rely on this variable, it will stop working unexpectedly.

@justmike1
Copy link
Contributor

why not just deleting this as a value and using {{ trimPrefix "v" .Values.global.datahub.version }} Instead? @jinlintt

@justmike1 For backward compatibility, we shouldn't delete any variables. Otherwise, for people who rely on this variable, it will stop working unexpectedly.

I see, I will also take this into consideration on my other PR #297

@jinlintt
Copy link
Contributor

@pedro93 could you take a look?
We know that we could set this in our values.yaml file. But if the chart has the matching cli version, then we have one less thing to worry and maintain.

We don't care as much when you released new version of CLI, but the chart doesn't match unless it has something important.

@github-actions
Copy link

This PR is stale. We will close it in 30 days if there is no comment or activity. If you want feedback but not able to get it on github please head to #contribute channel in slack at https://slack.datahubproject.io.

@github-actions github-actions bot added the stale label May 19, 2023
@TonyOuyangGit
Copy link
Contributor Author

Closing this PR, it's already updated by latest release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants