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

Test: phpstan to use config file #1448

Merged
merged 2 commits into from Sep 29, 2021

Conversation

yookoala
Copy link
Member

Description

  • Use phpstan.neon to config the behaviour instead of using the
    CLI arguments. Much cleaner and more readable.
  • Define how to format a .neon file in .editorconfig to ensure
    consistancy.

Motivation and Context

  • The old way misses all moduleFunctions.php when scanning for functions. Adding all those files to command argument would be a huge pain in the ass. Using a config file is more sensible.

How Has This Been Tested?

  • Locally.
  • CI environment.

* Use phpstan.neon to config the behaviour instead of using the
  CLI arguments. Much cleaner and more readable.
* Define how to format a .neon file in .editorconfig to ensure
  consistancy.
Copy link
Member

@SKuipers SKuipers left a comment

Choose a reason for hiding this comment

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

We have a lot of configuration-style files that are starting to clutter the root folder, I wonder if we could put the phpstan.neon file in a folder, such a /tests, to keep it out of the root, since it's part of the CI/test suite.

@yookoala
Copy link
Member Author

This should be do-able, but most projects leave these in the root.

@SKuipers
Copy link
Member

Hmm, I wonder if we try and get the release build process to clean these types of configuration files up, because I've been noticing there's more and more of them in the root. It's likely not good practice to have them in production servers, because many robots out there are often scraping for standard configuration files.

@yookoala
Copy link
Member Author

Sure. I think we can do 2 things in parallel:

  1. Add .htaccess to forbid access to those files.
  2. Update the release package building workflow to exclude those files.

Copy link
Member

@SKuipers SKuipers left a comment

Choose a reason for hiding this comment

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

Looking good 👍

@SKuipers SKuipers merged commit 736c26f into GibbonEdu:v23.0.00 Sep 29, 2021
@yookoala yookoala deleted the feat/phpstan-improved branch September 29, 2021 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants