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

CFE-3967: cfbs no longer tries to commit when there is no diff #187

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

mineralsfree
Copy link
Contributor

Ticket: CFE-3967
Changelog: None
Signed-off-by: Mikita Pilinka mikita.pilinka@northern.tech

@larsewi
Copy link
Contributor

larsewi commented Jan 24, 2024

Should get you closer to passing CI

black cfbs/*.py
git add cfbs/
git commit --amend --no-edit
git push -f

@mineralsfree mineralsfree force-pushed the CFE-3967 branch 2 times, most recently from e49afe4 to 5de3401 Compare January 25, 2024 11:35
@mineralsfree mineralsfree added the WIP Work in progress label Jan 25, 2024
@mineralsfree mineralsfree removed the WIP Work in progress label Jan 25, 2024
cfbs/git.py Outdated
Comment on lines 200 to 201
except CalledProcessError as cpe:
return False
Copy link
Member

Choose a reason for hiding this comment

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

When does this happen? Deserves at least a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might happen if, for example the command fired not in a git repository
I'm not sure what should be the expected behavior :)
Maybe it's better to raise Exception?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see.

In that case I'd probably do:

user_error("Failed to run 'git status -s -u' to check for changes.")

Also, please ensure that your change works for projects not using git committing ("git": false in cfbs.json).

cfbs/git.py Outdated
Comment on lines 190 to 195
changes = list(
map(
lambda m: m.strip().split(" ")[1],
list(filter(None, result.stdout.decode("utf-8").split("\n"))),
)
)
Copy link
Member

Choose a reason for hiding this comment

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

@mineralsfree something like this should work, and might be considered more readable :)

Suggested change
changes = list(
map(
lambda m: m.strip().split(" ")[1],
list(filter(None, result.stdout.decode("utf-8").split("\n"))),
)
)
lines = result.stdout.decode("utf-8").split("\n")
changes = [line.strip().split(" ")[1] for line in lines if line]

Ticket: CFE-3967
Changelog: None
Signed-off-by: Mikita Pilinka <mikita.pilinka@northern.tech>
Copy link
Contributor

@larsewi larsewi left a comment

Choose a reason for hiding this comment

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

🚀

@olehermanse olehermanse merged commit 5230ee3 into cfengine:master Feb 2, 2024
7 checks passed
@olehermanse olehermanse changed the title CFE-3967: cfbs should not try to commit without diff CFE-3967: cfbs no longer tries to commit when there is no diff Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants