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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use of deprecated set-output command #238

Merged
merged 10 commits into from Nov 3, 2022

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Nov 3, 2022

Description

As per #235:

set-output has been deprecated. See https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/.

Fixes #236
Closes #235

Motivation and context

This will remove deprecated notices in workflow summaries.

How has this been tested?

This has been tested on the one hand by actually running the tests and on the other hand, by verifying the syntax used in the additional adjustments I made via https://rextester.com/l/tcl_online_compiler

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR checklist

  • My change requires a change to the documentation.
    馃憠馃徎 I don't think so, but as per the discussion here First pass at removing set-output聽#235 (comment), the name of one of the (internal) outputs has changed from command to composer_command, but that output may possibly be exposed the action.
    If that's the case, this would need a changelog entry just in case anyone would be directly grabbing the output and re-using it in their workflow.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.

Commit info

.. for the commits added on top of the original PR #235:

composer_paths.sh: fix issue flagged by shellcheck

Ref: https://www.shellcheck.net/wiki/SC2129

Tests: fix inline comments

... and make them slightly more descriptive.

Tests/composer_paths_09.exp: fix expectation

Follow up to #237 which updated the composer.phar file, but missed the update needed to the test expectations.

Tests: fix test which need a pattern match

Note: there may well be a way to not have the regex inline, which would make the code more readable, but I'm not familiar enough with Tcl and to be honest, I'm already darn glad that I managed to get it working ;-)

Refs:

desrosj and others added 10 commits November 3, 2022 16:25
Bash gets confused with the variable being named `command`.
... and make them slightly more descriptive.
Follow up to 237 which updated the `composer.phar` file, but missed the update needed to the test expectations.
Note: there may well be a way to not have the regex inline, which would make the code more readable, but I'm not familiar enough with Tcl and to be honest, I'm already darn glad that I managed to get it working ;-)

Refs:
* https://www.tcl.tk/man/tcl8.5/TclCmd/regexp.html
* https://www.tcl.tk/man/tcl8.5/TclCmd/re_syntax.html
@jrfnl jrfnl requested a review from ramsey as a code owner November 3, 2022 19:11
@jrfnl jrfnl mentioned this pull request Nov 3, 2022
7 tasks
@jrfnl
Copy link
Contributor Author

jrfnl commented Nov 3, 2022

@ramsey Might be helpful for other contributors to include the links to the Tcl manual and the online testing tool in the CONTRIBUTING docs ;-)

@codecov
Copy link

codecov bot commented Nov 3, 2022

Codecov Report

Merging #238 (fbe8889) into v2 (72896eb) will decrease coverage by 0.83%.
The diff coverage is 88.88%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##               v2     #238      +/-   ##
==========================================
- Coverage   98.29%   97.45%   -0.84%     
==========================================
  Files           5        5              
  Lines         117      118       +1     
==========================================
  Hits          115      115              
- Misses          2        3       +1     
Impacted Files Coverage 螖
bin/composer_paths.sh 97.29% <80.00%> (-2.71%) 猬囷笍
bin/cache_key.sh 94.87% <100.00%> (酶)
bin/php_version.sh 100.00% <100.00%> (酶)
bin/should_cache.sh 100.00% <100.00%> (酶)

@ramsey
Copy link
Owner

ramsey commented Nov 3, 2022

Thanks, @jrfnl !

@ramsey ramsey merged commit 7f9021e into ramsey:v2 Nov 3, 2022
@jrfnl jrfnl deleted the feature/remove-set-output branch November 3, 2022 20:24
@desrosj
Copy link
Contributor

desrosj commented Nov 3, 2022

Thanks all!

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.

set-output function is deprecated
3 participants