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

Making reusable-regenerate-readme GitHub-y #53

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

szepeviktor
Copy link

@szepeviktor szepeviktor commented Jan 25, 2023

Use GHA outputs and true/false as values 🎉

-        if: env.CHANGES_DETECTED == 1
+        if: ${{ steps.check_changes.outputs.detected == 'true' }}

Please see comments below.

Some viktor utopia for consideration

  • quote every YAML string
  • indent with 4 space 👁️👁️

May I continue with other actions ❔

jobs:

regenerate-readme: #----------------------------------------------------------
Copy link
Author

@szepeviktor szepeviktor Jan 25, 2023

Choose a reason for hiding this comment

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

This may get emotional.
👨🏻‍🎨

Comment on lines -24 to +27
uses: shivammathur/setup-php@v2
with:
php-version: '7.4'
env:
COMPOSER_TOKEN: ${{ secrets.GITHUB_TOKEN }}
uses: shivammathur/setup-php@v2
with:
php-version: "7.4"
Copy link
Author

Choose a reason for hiding this comment

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

Put things in chronological order.
env is prerequisite.

custom-cache-suffix: $(date -u "+%Y-%m")
custom-cache-suffix: |
$(date --utc "+%Y-%m")
Copy link
Author

@szepeviktor szepeviktor Jan 25, 2023

Choose a reason for hiding this comment

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

Protect shell scripts. Use long options.

@schlessera
Copy link
Member

@szepeviktor Yes, happy to accept even more changes to get this to an idiomatic way of writing GHA workflows.

@danielbachhuber
Copy link
Member

Some viktor utopia for consideration

I think this might be a bit much 😁

The other changes seem fine. I'll defer to @schlessera for merge.

@szepeviktor
Copy link
Author

Now unit and feature test detection are separate jobs.

has_unit_tests:
runs-on: ubuntu-20.04
steps:
- name: Check out source code
uses: actions/checkout@v3
- name: Check existence of composer.json file
id: check_files
uses: andstor/file-existence-action@v2
with:
files: "composer.json, phpunit.xml.dist"
outputs:
exists: ${{ steps.check_files.outputs.files_exists }}

Could you test it? Then I will finish GitHubify the rest.

@danielbachhuber
Copy link
Member

@szepeviktor Thanks! I'll defer to @schlessera when he's available. My opinions about GitHub workflows are less strong/informed.

@szepeviktor
Copy link
Author

My opinions about GitHub workflows are less strong/informed.

I also use a cat+keyboard combo to send pull requests 🐈⌨️

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.

3 participants