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

Support sub chart dir #49

Closed
wants to merge 1 commit into from
Closed

Support sub chart dir #49

wants to merge 1 commit into from

Conversation

tuannvm
Copy link

@tuannvm tuannvm commented Oct 25, 2020

Why?

  • This change in PR will bring sub-directory support for charts_dir. Let's go through this example:
.
├── ops
│   ├── helm
│   │   ├── .helmignore
│   │   ├── chart1
│   │   ├── chart2
│   │   ├── chart3
│   │   ├── chart4
│   │   └── chart5

So, the charts_dir would be ops/helm

  • Before:
cut -d '/' -f "1,2" <<< "ops/helm/chart1" ## -----> ops/helm --> no chart found
  • After:
cut -d '/' -f "1-" <<< "ops/helm/chart1" ## -----> ops/helm/chart1 --> chart found

Signed-off-by: Tommy Nguyen tuannvm@hotmail.com

Signed-off-by: Tommy Nguyen <tuannvm@hotmail.com>
@unguiculus
Copy link
Member

Thanks for the PR. I think, this needs some more thought. The problem is that this will also detect local chart dependencies which not desireable. I'm not really sure if we should support your use case. Have you tried changing the working directory? Maybe that would work in your case.

@gabriellemadden
Copy link

Have you tried changing the working directory? Maybe that would work in your case.

Changing working directory does not fix the issue, as the output of git diff (called here: https://github.com/helm/chart-releaser-action/blob/master/cr.sh#L200) still returns a path from the root of the git repo no matter which subdirectory you're in. This PR handles nested charts_dir nicely: #47 - are there any plans to merge this?

@unguiculus
Copy link
Member

Closing this in favor of #47.

@unguiculus unguiculus closed this Oct 27, 2020
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.

4 participants