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

chore: correct step output key in readme #1057

Closed
wants to merge 2 commits into from

Conversation

willarmiros
Copy link
Contributor

Just a quick doc update - seems like the output keyname for a release being created has changed and it wasn't reflected in docs. See this output from release-please for evidence: https://github.com/willarmiros/test-package-lock-repo/runs/3617929254?check_suite_focus=true#step:3:4

@willarmiros willarmiros requested a review from a team September 16, 2021 05:54
@willarmiros willarmiros requested a review from a team as a code owner September 16, 2021 05:54
@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

@google-cla
Copy link

google-cla bot commented Sep 16, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Sep 16, 2021
@willarmiros
Copy link
Contributor Author

@googlebot I signed it!

@google-cla google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 16, 2021
@@ -173,19 +173,19 @@ jobs:
- uses: actions/checkout@v2
# these if statements ensure that a publication only occurs when
# a new release is created:
if: ${{ steps.release.outputs.release_created }}
if: ${{ steps.release.outputs.releases_created }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@willarmiros willarmiros Sep 22, 2021

Choose a reason for hiding this comment

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

I think we were both mistaken - it looks like releases_created is actually set as output when there is any release: https://github.com/google-github-actions/release-please-action/blob/973b63d3d58b9a5a9fed0dd512501ad5a9ab319b/index.js#L49

Whereas release_created wasn't removed as I initially thought, but is only set if you are releasing a package at the root of the repo(?) Regardless, IMO it would be more correct to use the output that is always set in the example code to cover more use cases.

@bcoe
Copy link
Contributor

bcoe commented Sep 23, 2021

@willarmiros are you using the manifest releaser, and this is why the releases_created is the variable you needed?

What about a compromise and updating the documents for the manifest functionality to include an example of the GitHub action?

@willarmiros
Copy link
Contributor Author

@bcoe ah yes I am using the manifest releaser, I guess the behavior is pretty different from the single-package option. I can try to add a sample action workflow to this PR in https://github.com/googleapis/release-please/blob/main/docs/manifest-releaser.md.

Given manifest releases would typically be for monorepos, would it be ok to include lerna in the example for running release-please and publishing to NPM?

@bcoe
Copy link
Contributor

bcoe commented Sep 24, 2021

Given manifest releases would typically be for monorepos, would it be ok to include lerna in the example for running release-please and publishing to NPM?

@willarmiros for sure, I'd be interested in seeing how you recommend doing this for my own benefit.

@bcoe
Copy link
Contributor

bcoe commented Oct 13, 2021

@willarmiros please feel free to dust this off if you'd like to add docs about lerna (I think this would be valuable for folks).

@willarmiros
Copy link
Contributor Author

@bcoe see #1100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants