Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle no-changed situation #77

Merged
merged 1 commit into from
May 19, 2023

Conversation

yo-ga
Copy link
Contributor

@yo-ga yo-ga commented May 17, 2023

Fix #76

git diff-index --quiet HEAD -- sometimes doesn't handle the no-changed situation.
Use git status --porcelain to handle modified, created, and deleted.

@jcbhmr
Copy link
Contributor

jcbhmr commented May 17, 2023

another option (not necessarily the one that we should go with 🤷‍♀️) is to just --allow-empty and ditch the "Are there any changes?" question

git add -A
git commit --allow-empty -m "$INPUT_COMMIT_MESSAGE"
#          ^^^^^^^^^^^^^

@yo-ga
Copy link
Contributor Author

yo-ga commented May 17, 2023

another option (not necessarily the one that we should go with 🤷‍♀️) is to just --allow-empty and ditch the "Are there any changes?" question

However, if we have already handled no-changed situation and exit from the script, there may be no situation to use allow-empty.

@yo-ga
Copy link
Contributor Author

yo-ga commented May 17, 2023

I think there is a potential issue about testing in different scenarios. For now, the test is for modified.

@jcbhmr
Copy link
Contributor

jcbhmr commented May 17, 2023

Here's a concrete demo of what I mean from an alternative GitHub wiki action:

https://github.com/Andrew-Chen-Wang/github-wiki-action/blob/jcbhmr/src/clone.sh#L90

# Allowing an empty commit is way easier than detecting empty commits! This also
# makes semantic sense. If you run this action, it adds a commit to your wiki.
# How large that commit is comes down to your changes. 0 change = commit with 0.
# This works well with the default "Update wiki ${{ github.sha }}" message so
# that even if the commit is empty, the message has the SHA there.
git commit --allow-empty -m "$INPUT_COMMIT_MESSAGE"

Copy link
Contributor

@jcbhmr jcbhmr left a comment

Choose a reason for hiding this comment

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

I think this looks like a good fix! Thanks for including the StackOverflow link too ❤️
@spenserblack is the owner with write perms, so we'll need to wait for a-OK from them.

@yo-ga
Copy link
Contributor Author

yo-ga commented May 17, 2023

If we use allow-empty, we can easily validate whether the wiki is kept up with the latest commit. Otherwise, we can easily know when the wiki is changed and what differences are between the wiki revision. It's up to you.

Copy link
Owner

@spenserblack spenserblack left a comment

Choose a reason for hiding this comment

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

LGTM!

As far as --allow-empty goes, IMO this would be some sort of input the user would have so that they would have the choice. So, in pseudocode, basically it would be this:

if git_has_changes OR input.allow_empty:
    commit()
else:
    report_empty()

@spenserblack spenserblack merged commit 57eed17 into spenserblack:main May 19, 2023
@spenserblack
Copy link
Owner

Oh, and as far as testing goes, TBH I'd want to switch over to using JS/TS before testing in-depth.

@yo-ga
Copy link
Contributor Author

yo-ga commented May 22, 2023

Hi @spenserblack, will a new version be released soon?

@yo-ga yo-ga deleted the bug/untrack-file branch May 22, 2023 08:10
@spenserblack
Copy link
Owner

@all-contributors add @yo-ga for code

@allcontributors
Copy link
Contributor

@spenserblack

I've put up a pull request to add @yo-ga! 🎉

@spenserblack
Copy link
Owner

https://github.com/spenserblack/actions-wiki/releases/tag/v0.3.0 🙂

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Not handle no-changed correctly
3 participants