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

PR Discussion - Manage uncommitted changes #49

Open
consideRatio opened this issue Oct 13, 2019 · 2 comments
Open

PR Discussion - Manage uncommitted changes #49

consideRatio opened this issue Oct 13, 2019 · 2 comments
Labels
enhancement New feature or request

Comments

@consideRatio
Copy link
Member

consideRatio commented Oct 13, 2019

If chartpress is run with unstaged changes that makes us require a image rebuild, what should we do?

Currently, the process chartpress works like...

  1. Get the last commit where dependent files for an image where changed.
  2. See if that image is available locally or at DockerHub and build it if it isn't.

So, if we have unstaged changes, chartpress will not rebuild or update the helm chart and images. I think it could be good to do this by default, but that the user may want to understand if this happens by being warned they need to commit those changes for chartpress to care.

I propose we add a check for unstaged changes of relevance and print a warning message about this. Oh no wait... chartpress could mess up badly unless we fail sometimes... Consider if it realize it requires a rebuild, but there are more unstaged changes of relevance for the image build. Then it could end up building a image for a commit tag that isn't representing the state of that commit.

We have to decide how we want chartpress to behave in at least two situations.

There are unstaged changes influencing an image, and...

  1. the tag is not available and needs to be rebuilt
  2. the tag is already available and doesn't need to be rebuilt

I think we should fail hard for 1), and for 2) the user at least needs to be informed.


Technically, to conclude if there are unstaged changes, we could use the currently unused (since 55fe935) function path_touched where we would pass the commit range of HEAD for it to catch staged and unstaged changes that has not yet been committed.

@minrk
Copy link
Member

minrk commented Oct 17, 2019

I think checking for uncommitted changes is great. I would try to avoid doing things too strict or complicated, and just append ".dirty" to versions if there are uncommitted changes in the inputs, probably also omitting a warning. I don't think failing hard is what we should do.

@minrk
Copy link
Member

minrk commented Oct 18, 2019

<pedant>I think it should be uncommitted changes we need to deal with. Staged or not shouldn't be relevant.</pedant>

@consideRatio consideRatio changed the title PR Discussion - Manage unstaged changes PR Discussion - Manage uncommitted changes Oct 21, 2019
@consideRatio consideRatio added the enhancement New feature or request label Oct 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants