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

fix: wrap assignment with quotes #1335

Merged
merged 1 commit into from
Nov 10, 2022
Merged

Conversation

kopackiw
Copy link
Contributor

@kopackiw kopackiw commented Nov 10, 2022

After merging the PR that removes deprecated set-otput, I saw that RELEASE_DETECTED flag is set to an empty string (it should be "false" or "true"). Thus, weekly official release might not work.

I think that problem might lie in quoting. If the flag on CD after merge still be an empty string, please @hannobraun revert this and the former PR to not disrupt your release pattern. Sorry for trouble 🙏🏻

Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thank you for this pull request, @kopackiw! Good catch on the variable being empty. I had a look myself, but missed this.

Sorry for trouble 🙏🏻

Please, no need to apologize! I'm very thankful that you're taking care of this. It might only be a small thing, but it's one of a million little things that I don't have time for, so again, thank you!

The risk of disrupting the release process is acceptable. As I said earlier somewhere, I think it would be a big pain to set up proper testing for this. It's just unreasonable to expect this, when we have a weekly release process that can serve as the perfect testing ground.

If any issues are still present after this, they'll get ironed out over the next few weeks. No big deal!

@hannobraun hannobraun merged commit 7b35c82 into hannobraun:main Nov 10, 2022
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