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

[Part 2] Re-factor publishing charts - gh-pages #50

Closed
wants to merge 22 commits into from

Conversation

ljubon
Copy link
Collaborator

@ljubon ljubon commented Aug 17, 2021

Old PR: [Part 2] Re-factor publishing charts - gh-pages #39

Due to cleaning up and dependency over Armada, I'm replacing source branch from:

old ljubon:gh-pages => ljubon:refactor-gh-pages new

==========================================

The goal of this PR is to separate gh-pages and source code a.k.a charts and re-use the same workflow to publish all charts across G-Research OSS repos

  • master branch
    • place for source code 🙏 (e.g charts, values..etc)
  • gh-pages branch
    • all public related stuff (e.g index.yaml for repo, raw chart files...etc)

NOTE:

  • Input for src_path and dst_path in REPO_CHART must end with /
    • e.g src_path": "deployment/helm-k8s/", "dst_path": "siembol/"

Here you can find example run for publishing storm charts
Input: publish-storm.yaml on master branch (e.g WIP-refactor-master)
Action: publish-charts.yaml triggered with given inputs (e.g WIP-refactor-gh-pages)
Output:

Same for Armada and Siembol
Siembol Input -> Charts output
Armada input -> Charts output

NOTE: 2nd PR #50

Copy link
Contributor

@76creates 76creates left a comment

Choose a reason for hiding this comment

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

looking good to me

@ljubon ljubon force-pushed the refactor-gh-pages branch 3 times, most recently from f0e2f17 to 51d5310 Compare August 26, 2021 00:48
@ljubon ljubon changed the base branch from master to gh-pages August 26, 2021 20:51
@ljubon ljubon force-pushed the refactor-gh-pages branch 2 times, most recently from 6a465eb to dc3a7b7 Compare August 26, 2021 20:52
@ljubon ljubon marked this pull request as ready for review August 26, 2021 20:54

- id: check
name: 'Check if chart already exist'
if: ${{ steps.chart-version.outcome == 'success' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

does this rely only on if clause or we actually need to have Chart.yaml and/or values.yaml found as well? If im not mistaken find wont exit 1 if it doesent match any files


- id: package
name: Package ${{ matrix.chart }}-${{ env.REPO_REF }}.tgz
if: ${{ steps.chart-version.outcome == 'success' && steps.check.outcome == 'success' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

steps.chart-version.outcome == 'success' is redundant as its needed for steps.check.outcome to be success


- id: publish
name: 'Push ${{ matrix.chart }}-${{ env.REPO_REF }}.tgz to gh-pages'
if: ${{ steps.package.outcome == 'success' }}
Copy link
Contributor

@76creates 76creates Aug 26, 2021

Choose a reason for hiding this comment

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

looks to me that these if clauses in check, package, and publish are unecesarry if we were to use continue-on-error on checkout-gh-pages and checkout-helm-src only, that seams like a proper step

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.

2 participants