successComment deprecation warning is logged for releases with no commits, even when successComment is not set
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 82/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- javascript
- Domain
- release
Research direction
Start in lib/success.js at the success() entry point and inspect the branch combining successComment === false with an empty commits list. Reproduce the behavior with commits: [] and no successComment option, then add a regression test showing that only an explicitly false successComment logs the deprecation warning while empty commits still log their normal messages.
Written by the indexing model from the issue text.
Description
On 12.0.9 (same code on master), lib/success.js logs the "'false' for 'successComment' is deprecated" warning for any release with an empty commits list, even when successComment was never set.
if (successComment === false || isEmpty(commits)) {
if (isEmpty(commits)) {
logger.log("No commits found in release");
}
logger.log("Skip commenting on issues and pull requests.");
logger.warn(
`DEPRECATION: 'false' for 'successComment' is deprecated and will be removed in a future major version. Use 'successCommentCondition' instead.`,
);
}
Both halves of the condition end up in the same branch, so the logger.warn runs for the empty-commits case too. We hit it through a monorepo release tool that creates releases for packages whose path-filtered commit list is empty (dependency-only releases). Nothing in our config sets successComment or successCommentCondition, and the warning shows up once per such release.
Following the advice in the message doesn't help either: the first branch is taken before successCommentCondition is read, so setting it to false or a condition changes nothing about the warning.
To reproduce, call success() with commits: [] and no successComment option. You get the "No commits found in release" log line followed by the deprecation warning.
I think the warning should only be logged when successComment === false. Happy to send a PR for that with a test if that's welcome.
- Dominant language
- JavaScript
- Stars
- 535
- Forks
- 150
- Avg merge
- 1m
- Merged PRs (30d)
- 3
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from semantic-release/github
-
Difficulty 4/5 3-5 days Newbie friendliness 38/100
semantic-release/github#1242 · 1 comment · 4 reactions ·
-
Difficulty 3/5 1-2 days Newbie friendliness 45/100
semantic-release/github#1103 · 1 comment · 1 reaction ·
-
bug
Difficulty 3/5 1-2 days Newbie friendliness 68/100
semantic-release/github#1092 · 6 comments ·
-
immutable releases Open
Difficulty 4/5 3-5 days Newbie friendliness 42/100
semantic-release/github#1082 · 5 comments · 2 reactions ·
-
bug Good first issue help wanted
Difficulty 3/5 1-2 days Newbie friendliness 55/100
semantic-release/github#1065 · 5 comments · 1 reaction ·
All issues in semantic-release/github
Similar issues
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
avniproject/avni-client#2135 ·
-
automated broken-link
Difficulty 1/5 Under an hour Newbie friendliness 85/100
-
agent/security hive/hosted-available-lke648397-260827-5n31 security
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
babalae/bettergi-scripts-list#3674 ·
-
A-Release-Notes C-Editing D-Modest S-Ready-For-Implementation
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
bevyengine/bevy-website#2595 ·