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

[MRG] Add top level path back to chartpress context to fix build issue #1014

Merged
merged 1 commit into from Nov 28, 2019

Conversation

@betatim
Copy link
Member

betatim commented Nov 28, 2019

Without this wider context we sometimes produce an old chart version
with a new image inside.

This is a follow up for #1004 where we removed this but shouldn't have. Was spotted in #919 (comment)

cc @bitnik @consideRatio

Without this wider context we sometimes produce an old chart version
with a new image inside.
@betatim

This comment has been minimized.

Copy link
Member Author

betatim commented Nov 28, 2019

Sorry for reintroducing this @bitnik and thanks for spending time to find it.

@consideRatio

This comment has been minimized.

Copy link
Member

consideRatio commented Nov 28, 2019

Wait what about: #919 (comment)

@bitnik

This comment has been minimized.

Copy link
Collaborator

bitnik commented Nov 28, 2019

I think we can merge this for now. And revert it later when there is a new version of chartpress (which includes the fix).

@betatim

This comment has been minimized.

Copy link
Member Author

betatim commented Nov 28, 2019

@jupyterhub/binder-team I added @bitnik as a collaborator to this repository. He helps a lot with many things but the one thing he can't help with is merging PRs :D Now he can!

@bitnik use your new power wisely.

@bitnik

This comment has been minimized.

Copy link
Collaborator

bitnik commented Nov 28, 2019

Thank you! I do it happily :)

@bitnik bitnik merged commit c1bee7e into jupyterhub:master Nov 28, 2019
4 checks passed
4 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
codecov/patch Coverage not affected when comparing 2a7145f...00f7197
Details
codecov/project 80.46% (target 0%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@betatim betatim deleted the betatim:fix-chartpress-context branch Nov 28, 2019
yuvipanda pushed a commit to jupyterhub/helm-chart that referenced this pull request Nov 28, 2019
Copy link
Member

consideRatio left a comment

Oh forgot to press send on this review

@@ -6,6 +6,10 @@ charts:
repo:
git: jupyterhub/helm-chart
published: https://jupyterhub.github.io/helm-chart
# We need the broader context to make sure we do not produce a chart
# tagged with an older version that contains a newer image

This comment has been minimized.

Copy link
@consideRatio

consideRatio Nov 28, 2019

Member

Add a note that this is not intentional chartpress behavior and can be fixed post 0.4.3!

consideRatio added a commit to consideRatio/binderhub that referenced this pull request Dec 1, 2019
Chartpress 0.5.0 contains a bugfix that makes a chart's images' context
paths be considered when determening the version of the chart as well as
the chart folder. This was not done before and caused issues, that made
us require jupyterhub#1014 which can
now be reverted in this commit.
consideRatio added a commit to consideRatio/binderhub that referenced this pull request Dec 2, 2019
Chartpress 0.5.0 contains a bugfix that makes a chart's images' context
paths be considered when determening the version of the chart as well as
the chart folder. This was not done before and caused issues, that made
us require jupyterhub#1014 which can
now be reverted in this commit.
consideRatio added a commit to consideRatio/binderhub that referenced this pull request Dec 2, 2019
Chartpress 0.5.0 contains a bugfix that makes a chart's images' context
paths be considered when determening the version of the chart as well as
the chart folder. This was not done before and caused issues, that made
us require jupyterhub#1014 which can
now be reverted in this commit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.