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 post-update-cmd with --no-dev installs #1067

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

Conversation

HenkPoley
Copy link

@HenkPoley HenkPoley commented Sep 23, 2020

Summary

When barryvdh/laravel-ide-helper is a dev dependency, it is not available with --no-dev installs ("release builds"). Which will give errors if you add ide-helper:<..> in post-update-cmd or post-install-cmd in composer.json.

This fixes that by making an intermediate that checks for APP_ENV="local" in .env to automatically run ide-helper scripts.

This patch is related to: #794

Type of change

  • 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 not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@HenkPoley
Copy link
Author

HenkPoley commented Sep 23, 2020

It does not check if the database is available.

Somebody may want to implement something along the lines of:

        try {
            \Illuminate\Support\Facades\DB::connection()->getPdo();
        } catch (Exception $e) {
            // If we get here we are not connected
            $this->comment($this->signature . ': No database connection, not updating PhpStorm ide-helper:* files.');
        }

Our setup connects to multiple databases, and as dev you'd rather see it break than become silent.

@HenkPoley HenkPoley changed the title Fix --no-dev installs Fix post-update-cmd with --no-dev installs Sep 23, 2020
@HenkPoley
Copy link
Author

Potentially ide-helper could drop such a file? I believe there are recommended ways to 'install files' in Laravel, e.g. to install config/<..>.php

Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

No objections from my side; having it doc only seems useful

Let's see what @barryvdh thinks!

}
```

In `composer.json` you can then add `"@php artisan ide-helper:do-it"` that will gracefully work indepent of `--no-dev` composer installs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it should be more explicit where to add this, as they're multiple keys possible in scripts, i.e. mention post-update-cmd ?

@HenkPoley
Copy link
Author

HenkPoley commented Sep 23, 2020

I see class_exists() could also be a nice test.

Is there some Laravel-ish, or Composer-ish way to see if ide-helper is loaded? And which class to test?

@netpok
Copy link
Contributor

netpok commented Mar 16, 2021

This should check whether ide-helper is installed:

Artisan::command('ide-helper:run', function () {
    if (App::has('command.ide-helper.generate')) {
        Artisan::call('ide-helper:meta', [], $this->output);
        Artisan::call('ide-helper:generate', [], $this->output);
    }
})->describe('Run ide-helper:generate and meta.');

This can be inserted to routes/console.php

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.

4 participants