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-4081: Fixed commiting of changes made by set-input #144

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

olehermanse
Copy link
Member

@olehermanse olehermanse commented Nov 22, 2022

More work remains to fix committing in cfbs add.

@olehermanse olehermanse added the WIP Work in progress label Nov 22, 2022
Should fail until I fix the bug.

Signed-off-by: Ole Herman Schumacher Elgesem <ole.elgesem@northern.tech>
Signed-off-by: Ole Herman Schumacher Elgesem <ole.elgesem@northern.tech>
@olehermanse olehermanse removed the WIP Work in progress label Nov 22, 2022
@larsewi larsewi changed the title Fixed commiting of changes made by set-input CFE-4081: Fixed commiting of changes made by set-input Nov 22, 2022
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.

ACK

@larsewi larsewi merged commit 5499450 into cfengine:master Nov 22, 2022
@olehermanse olehermanse deleted the commits branch November 22, 2022 12:39
git diff --exit-code --staged

# Error if there are uncommited changes (to tracked files):
git diff --exit-code
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like these checks should be in a nicely-named function in some shared bash source.

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, good idea

old_data = read_json(path)
if old_data == data:
log.debug("Input data for '%s' unchanged, nothing to write / commit" % name)
return 0
Copy link
Contributor

Choose a reason for hiding this comment

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

This should, ideally, be done before the more expensive checks/comparison above.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking it makes sense to always do the expensive checks (validation), even if the (potentially bad) data is already there.

if config.get("git", False):
git_commit(
"Set the input for module '%s'" % name,
edit_commit_msg=False,
Copy link
Contributor

Choose a reason for hiding this comment

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

No interactivity here and prompting the user if they want to modify the commit message?

Copy link
Member Author

@olehermanse olehermanse Nov 22, 2022

Choose a reason for hiding this comment

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

Right, I am submitting some changes which should explain / clear this up. (In a follow-up).

Copy link
Member Author

Choose a reason for hiding this comment

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

WIP: #145

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