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

Add justfile, remove coveralls, and fix AUTOLOAD in CI #1801

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

xavdid-stripe
Copy link
Member

@xavdid-stripe xavdid-stripe commented Jan 8, 2025

Why?

In an effort to modernize and simplify our local tooling, we're moving our dev commands from makefiles to justfiles. This is intended to be mostly a drop-in replacement, but some command names may change per standardization efforts.

Namely, I renamed fmt to format, added lint in place of phpstan, and swapped vendor to install.

One actual change is that dependencies won't auto-install before running tests like they did before. Users will have to just install before just test.

Also, I didn't add a ton of docstrings, since the recipes are mostly self explanatory. But, we could add some. The recipes are also grouped, so the help output looks like:

Available recipes:
    [useful]
    install            # install vendored dependencies; only used locally
    test *options
    format *options

    [CI]
    ci-test
    format-check
    lint *options

    [misc]
    phpstan-baseline
    phpdoc

Without groups, output of just is:

Available recipes:
    install            # install vendored dependencies; only used locally
    test *options
    ci-test
    format *options
    format-check
    lint *options
    phpstan-baseline
    phpdoc

Which isn't bad either. I could go either way on the groups. If nothing else, I think it makes the justfile itself harder to grok, so i'm fine to pull them too.

CI

Also, there was an issue where we weren't correctly setting the AUTOLOAD env var, so we were never running tests with autoload=1. The justfile flagged this issue and I changed the way the matrix was loaded to acommodate it.

What?

  • added justfile
  • added warning to top of makefile
  • tweaked CI to use just instead of make
  • fixed CI so AUTOLOAD was correctly set
  • removed PHPstan from a matrix and just hardcoded the version we wanted.

See Also

composer.json Outdated Show resolved Hide resolved
@xavdid-stripe xavdid-stripe changed the title Add justfile, remove coveralls Add justfile, remove coveralls, and fix AUTOLOAD in CI Jan 8, 2025
- uses: actions/checkout@v3

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php-version }}
php-version: "8.2"
Copy link
Member Author

Choose a reason for hiding this comment

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

Since we're not running phpstan across multiple PHP versions, I just hardcoded the value we're using

runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
env:
Copy link
Member Author

Choose a reason for hiding this comment

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

this never worked as intended: env doesn't actually set the environment, it just sets a github variable called env

env:
- AUTOLOAD=0
- AUTOLOAD=1
autoload:
Copy link
Member Author

Choose a reason for hiding this comment

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

because just recipes can take arguments, we can replace the env-looking value with the actual value we want and pass it in directly

@@ -92,15 +86,16 @@ jobs:
- "8.0"
- "8.1"
- "8.2"
name: Tests (php@${{ matrix.php-version }}, AUTOLOAD=${{ matrix.autoload }})
Copy link
Member Author

Choose a reason for hiding this comment

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

This will show a nicer name now that we're not using the full AUTOLOAD string

@xavdid-stripe
Copy link
Member Author

Because the names of the CI jobs changed, there's a mismatch on the required tests. We'll have to bypass some branch protections in PHP during the cutover, but should be smooth sailing after that.

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