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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(manifest): add option to skip creating github release #1048
Conversation
@rabraghib this looks quite reasonable to me, will let @joeldodge79 chime in too. |
thanks @bcoe for your feedback!! |
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.
sorry for the lag (missed the notification). Thanks for the contribution, I like the feature! I have a suggestion (inline) for slightly different behavior, let me know what you think.
Also, could you add a test or two?
src/manifest.ts
Outdated
[{sha: lastMergedPR.sha, message: '', files: lastMergedPR.files}], | ||
lastMergedPR.sha | ||
) | ||
).filter(pkg => !pkg.config.skipGithubRelease); |
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.
What do you think about a slightly different approach: rather than just exclude them what if we checked this flag further down and did this
this.gh.commentOnIssue("`:robot: ${pkgName} not configured for release :no_entry_sign:`")
Easier for after-the-fact troubleshooting (rather than looking at git history on release-please-config.json
and tying the timeline to release creation runs)
I also think it handles this edge case better: all packages are marked as skip-github-release
- that way we still "release" the PR (remove the autorelease: pending
label and add the autorelease: tagged
label). That doesn't really affect functionality (we only ever look at the lastMergedPR
) but for historical viewing/reporting I think it makes more sense to see a "released" PR and it just so happens that no package was actually released.
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.
Yp I totally agree.
I will update it & I will try adding some tests
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.
Looks good! seems like there's a file conflict that needs fixing though
@joeldodge79 fixed. |
@rabraghib thank you for the contribution. |
馃 I have created a release \*beep\* \*boop\* --- ## [12.3.0](https://www.github.com/googleapis/release-please/compare/v12.2.0...v12.3.0) (2021-09-24) ### Features * allow forcing latest tag ([#1070](https://www.github.com/googleapis/release-please/issues/1070)) ([0549a30](https://www.github.com/googleapis/release-please/commit/0549a3035c8348c62958d2f1f037226bf2a0ce21)) * **manifest:** add option to skip creating github release ([#1048](https://www.github.com/googleapis/release-please/issues/1048)) ([59f3094](https://www.github.com/googleapis/release-please/commit/59f309429586200f835fdffe07ed9860a1901e31)) * **python:** support src/packagename/__init__.py ([#1062](https://www.github.com/googleapis/release-please/issues/1062)) ([598667d](https://www.github.com/googleapis/release-please/commit/598667da5a623c3fb057874840b3c308d225c627)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #1042 馃
Here is a simple repo that shows the functionality: https://github.com/rabraghib/release-please-tests