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

fix(target) do not open a PR when a target does not change a file #377

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

dduportal
Copy link
Contributor

@dduportal dduportal commented Nov 17, 2021

Fix #375

The problem was caused because the "if block" responsible to open the pull request was not position in the correct location.

To fix this issue, we are returning early when a target does not change anything: it removes 1 layer of "if/else" and it ensures that the "if" block responsible for PR creation is NEVER called when there isn"t any change.
Also, the "if block" had been put in the most specific if branch to avoid repeated conditions (!dry run and o.push).

=> IF it causes logic trouble in the cinematic for opening PR, then it means that addition refactoring will be required to ensure that the concept of "happy path" + "early return" are met. Unit tests would greatly help in this case.

Verified against https://github.com/dduportal/charts:

$ updatecli --debug apply --values ./updatecli/values.yaml --config ./updatecli/updatecli.d/charts/acme.yaml
# ...
TARGETS
========

updateChartVersion
------------------
DEBUG: stage: git-checkout

DEBUG: checkout branch "updatecli_08bce438947f81e32d2fcf5d995be35020abe6ae73a1d04bad797f3489031748", based on "main" to directory "/var/folders/2s/09szbrgn22l2tcvxz1_k34g40000gn/T/updatecli/dduportal/charts"
DEBUG: branch 'updatecli_08bce438947f81e32d2fcf5d995be35020abe6ae73a1d04bad797f3489031748' doesn't exist, creating it from branch 'main'
DEBUG: branch "updatecli_08bce438947f81e32d2fcf5d995be35020abe6ae73a1d04bad797f3489031748" successfully created
✔ Content from file "/var/folders/2s/09szbrgn22l2tcvxz1_k34g40000gn/T/updatecli/dduportal/charts/clusters/publick8s.yaml" already up to date

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

REPORTS:


✔ ACME.YAML:
        Sources:
                ✔ [lastChartVersion] (helmChart)
        Target:
                ✔ [updateChartVersion]  Update the chart version for acme(file)

@dduportal dduportal added this to the 0.15.0 milestone Nov 17, 2021
@dduportal dduportal added bug Something isn't working target Related to updatecli target labels Nov 17, 2021
lemeurherve
lemeurherve previously approved these changes Nov 17, 2021
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working target Related to updatecli target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SCM target] error message when no changes to be applied
2 participants