-
Notifications
You must be signed in to change notification settings - Fork 1
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
Sync fork #3
Sync fork #3
Conversation
fixes for: suffix.X in pre-tags bumps & refarctor some of the shell logic
bump major also
…ick#244) * validate default_branch when history type full * This var is branch_history * there is no head in the container hence we change the strategy for filtering * Add readme warning when combining history full without specifying the default branch an is not main or master
used more recent version for my trials, figured that 1.55.0, which is mentioned in the README will not respect commit keywords and always default to the DEFAULT_BUMP. version 1.61.0 does not, therefore better example IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
actionlint
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:75:19: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2086:info:75:41: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2086:info:75:44: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:79:55: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:80:52: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:82:55: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:83:52: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:85:59: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:86:59: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2015:info:88:55: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2086:info:92:54: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml|107 col 9| shellcheck reported issue in this script: SC2086:info:94:49: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml|107 col 42| property "old_tag" is not defined in object type {new_tag: string; part: string; tag: string} [expression]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2129:style:28:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:28:51: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:30:47: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:31:54: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:32:49: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:64:49: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:66:47: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:67:54: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:68:49: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2006:style:72:14: Use $(...) notation instead of legacy backticks ...
[shellcheck]
Test pipeline seems to fail because of this being a fork. At least there's no change to the original repo that could cause it. |
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2015:info:75:19: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:75:41: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:75:44: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2015:info:79:55: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2015:info:80:52: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2015:info:86:59: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2015:info:88:55: Note that A && B || C is not if-then-else. C may run when A is true [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:92:54: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
shellcheck reported issue in this script: SC2086:info:94:49: Double quote to prevent globbing and word splitting [shellcheck]
.github/workflows/test.yml
Outdated
# Check if the action created the expected output | ||
- name: Check if the tag would have been created | ||
shell: bash | ||
run: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[actionlint] reported by reviewdog 🐶
property "old_tag" is not defined in object type {new_tag: string; part: string; tag: string} [expression]
|
||
current_branch=$(git rev-parse --abbrev-ref HEAD) | ||
|
||
pre_release="false" # never use pre_release since we want this to run on all branches | ||
pre_release="$prerelease" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
already default false
in new line 16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Teardown for a test PR in push-api repo worked well (CircleCI run). Could still be we have weird edge cases but normal use-case seems to be fine. So I'd merge it soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you spot anything fishy? I didn't. But it is quite a complex project compared to what we want to achieve with it.
The action include another 3rd party action dependency (seems to be responsible for commenting with bash syntax hints). If you don't need it I'd recommend to disable that step.
I mainly checked the You have a very good point though. There's a few third-party-dependencies in the GitHub workflows that run in this repo if we work on the code (e.g. the linter you mentioned; so not if the actual action gets executed). I'll just delete them as we indeed don't need them. That should still allow for easy updating in the future. But overall it's indeed a big repo now for the simple use case we have. I'd still suggest to not manually maintain this in the future - at least at this point. |
In an effort to get rid of the deprecated usages of
::set-output
(Trello ticket), this is the sync to the original repo where it has been fixed already.I'm adding an info to the README as well on why we went for the fork (see also slack discussion).