Skip to content

Use correct header in Helm chart #1560

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbonofre
Copy link
Member

The header doesn't seem correct here. It should be the ASF header as it's a Polaris resource.

@adutra can you please confirm ?

@jbonofre
Copy link
Member Author

@justinmclean as discussed on the incubator general mailing, this PR fixes the mistake on the header (the Helm chart yaml has been created in Apache Polaris (incubating)).

flyrain
flyrain previously approved these changes May 10, 2025
Copy link
Contributor

@flyrain flyrain left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @jbonofre !

@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board May 10, 2025
@adutra
Copy link
Contributor

adutra commented May 11, 2025

@adutra can you please confirm ?

@jbonofre this is OK for now but in fact, we don't have spotless rules for YAML files. I can open a PR for that, but spotless + YAML is tricky, we will probably need to use ## for the license header.

adutra
adutra previously approved these changes May 11, 2025
@jbonofre
Copy link
Member Author

I disabled spotless in another PR because we have different headers to keep.

Here the question is: this file has been created in Polaris. I guess yes but wanted to double check with you 😀

@adutra
Copy link
Contributor

adutra commented May 11, 2025

Here the question is: this file has been created in Polaris. I guess yes but wanted to double check with you 😀

It was copied from Nessie.

In this folder, the following files are copied from Nessie:

  • logging_storage_test.yaml
  • quantity_test.yaml
  • service_monitor_test.yaml

In general, I'd say around 70% of the Polaris Helm chart is a copy from Nessie Helm chart.

dimas-b
dimas-b previously approved these changes May 13, 2025
@jbonofre
Copy link
Member Author

@adutra ok, thanks, if it's copied from Nessie, this PR is not good because:

  1. It should be documented in LICENSE
  2. The header should be the Nessie one

I'm updating the PR. Thanks !

@jbonofre jbonofre dismissed stale reviews from dimas-b, adutra, and flyrain via da0aa86 May 14, 2025 13:36
@jbonofre jbonofre force-pushed the fix-helm-logging-storage-test-asf-header branch from f05a255 to da0aa86 Compare May 14, 2025 13:36
@jbonofre jbonofre requested a review from singhpk234 as a code owner May 14, 2025 13:36
@jbonofre jbonofre requested review from adutra, flyrain and dimas-b May 14, 2025 13:36
@jbonofre
Copy link
Member Author

@adutra I compared Nessie and Polaris helm to find the files with more than 70% in common (same code). I updated the header and the LICENSE file. Can you please take a look ?

adutra
adutra previously approved these changes May 14, 2025
@jbonofre jbonofre force-pushed the fix-helm-logging-storage-test-asf-header branch from da0aa86 to 237e157 Compare May 14, 2025 16:46
@RussellSpitzer
Copy link
Member

Minor note, we should probably change the PR title to reflect we are restoring to Dremio now.

RussellSpitzer
RussellSpitzer previously approved these changes May 14, 2025
@jbonofre jbonofre changed the title Fix ASF header in helm chart Use correct header in Helm chart May 14, 2025
@jbonofre jbonofre force-pushed the fix-helm-logging-storage-test-asf-header branch from 237e157 to aa44d99 Compare May 14, 2025 19:02
@jbonofre
Copy link
Member Author

@RussellSpitzer did it, thanks for the suggestion !

@jbonofre jbonofre added this to the 0.10.0-beta milestone May 14, 2025
@jbonofre jbonofre dismissed stale reviews from adutra and RussellSpitzer via f50d153 May 15, 2025 07:06
@jbonofre jbonofre force-pushed the fix-helm-logging-storage-test-asf-header branch from aa44d99 to f50d153 Compare May 15, 2025 07:06
@jbonofre jbonofre requested review from RussellSpitzer and adutra May 15, 2025 07:46
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.

5 participants