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

Support custom git params #15

Conversation

gomorizsolt
Copy link
Contributor

Issue #14.

IMO it'd make sense to default commit_options to an empty string but it's still an issue we're facing with, see #12 (comment). As a temporary workaround, how about creating a get_value function?

Here's an example how it'd look like in JavaScript.

get_value(value, defaultValue) {
   if (typeof value !== "undefined" && value !== null) {
      return value;
   }

   return defaultValue;
}

This generic function improves on code's reusability and readability.

@gomorizsolt
Copy link
Contributor Author

gomorizsolt commented Oct 31, 2019

Though I'm uncertain whether it's the correct syntax, here's a more comprehensive example.

get_value () {
   if [ -z {$1+x} ];
   then
      return $2
   else
      return $1
}

# ...

git add get_value $INPUT_FILE_PATTERN '.'

However, I assume this won't work if $INPUT_FILE_PATTERN is undefined(or whatever it's called in bash), see our discussion here. I'd therefore propose to find the root cause of the issue instead.

@stefanzweifel
Copy link
Owner

Thanks!
A function would be really helpful. Have to be honest here: Haven't used them much yet.

Will do some research and also trying to figure out, how we can tell GitHub Actions to resepect the default values for input options.

@stefanzweifel stefanzweifel self-requested a review October 31, 2019 09:52
@gomorizsolt
Copy link
Contributor Author

I've stumbled upon Publish-Docker-Github-Action which defines a uses() function to check whether the input param is set, so it'd make sense to stick to the suggested temporary workaround, though it's still an unexpected behavior.

@gomorizsolt
Copy link
Contributor Author

gomorizsolt commented Oct 31, 2019

Refer to b6dcf94. What are your thoughts on this approach? Would it make sense to make get_value less error-prone? E.g. when neither of the values are defined print a message to the console and exit 1.

Another approach would be to use solely the is_defined function similarly to your solution.

is_defined() {
    [ ! -z "${1}" ]
}

if is_defined "${INPUT_FILE_PATTERN}"; then
   git add  "${INPUT_FILE_PATTERN}"
else
   git add .
fi

if is_defined "${INPUT_COMMIT_OPTIONS}"; then
   git commit -m "$INPUT_COMMIT_MESSAGE" --author="$GITHUB_ACTOR <$GITHUB_ACTOR@users.noreply.github.com>" "${INPUT_COMMIT_OPTIONS}"
else
   git commit -m "$INPUT_COMMIT_MESSAGE" --author="$GITHUB_ACTOR <$GITHUB_ACTOR@users.noreply.github.com>"
fi

It might be a bit more readable and less error-prone.

@stefanzweifel
Copy link
Owner

Would it make sense to make get_value less error-prone? E.g. when neither of the values are defined print a message to the console and exit 1.

Good (and hard) question. I don't know if there's a common convention in bash scripts of what should happen if arguments are missing.

Also see next question/answer ↓.

Another approach would be to use solely the is_defined function similarly to your solution.

I personally like that a bit more. The code looks a bit cleaner and it's immediately clear what the code is doing.

So could you update the script with your if-else-code?


Have you tested your PR within GitHub Actions already? As far as I know, there's currently no easy way to test Actions locally.
(I have some ideas for automated testing, but didn't have the time yet to build it)

@gomorizsolt
Copy link
Contributor Author

So could you update the script with your if-else-code?

Sure, check out af33cfb.

Have you tested your PR within GitHub Actions already?

Not quite, but I've already set up a test repo while I was trying to figure out why the Action doesn't respect default params.

@gomorizsolt
Copy link
Contributor Author

Have you ever had such errors?

https://github.com/gomorizsolt/git-auto-commit-action-test-repo/commit/a27ce8f8ae3909482619b107916d566cace4dad5/checks?check_suite_id=290066630

Not a git repository
To compare two paths outside a working tree:
usage: git diff [--no-index] <path> <path>
fatal: not a git repository (or any parent up to mount point /github)

What do I miss?

@gomorizsolt
Copy link
Contributor Author

Sorry, my bad, forgot to use actions/checkout@v1.

entrypoint.sh Outdated Show resolved Hide resolved
entrypoint.sh Outdated Show resolved Hide resolved
@stefanzweifel stefanzweifel merged commit e924b16 into stefanzweifel:master Nov 4, 2019
@stefanzweifel
Copy link
Owner

v2.3.0 has been released.

Thanks again!

@gomorizsolt gomorizsolt deleted the issue-14_support-custom-git-params branch November 4, 2019 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants