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

10 3 #26

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

10 3 #26

wants to merge 19 commits into from

Conversation

agentrickard
Copy link
Contributor

  • Updates to PHP 8.3
  • Updates to Drupal 10.3
  • Adds additional commands

Testing

  • Follow the instructions in the README and ensure that they work

Copy link

@sarahwylie sarahwylie left a comment

Choose a reason for hiding this comment

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

I pulled down a fresh install of the repo, ran the commands presented in the README, and was able to spin up the site successfully! I looked through the code differential, and I did a search for "8.2".

Feedback: Line 117 of the README still lists the default PHP version as 8.2:
Use the '-v' flag to specify a PHP version to test. By default, the version is '8.2'

I ran the tests laid out in the README against admin_toolbar. Results below:

  • check: Time: 169ms; Memory: 8MB (Success)
  • cspell: CSpell: Files checked: 69, Issues found: 146 in 22 files
  • compat: Time: 160ms; Memory: 8MB (Success)
  • eslint: ✖ 261 problems (212 errors, 49 warnings), Failed to run eslint admin_toolbar: exit status 1
  • md: Failed to run md admin_toolbar: exit status 2 (Over the threshold on several tests)
  • rector: [OK] Rector is done!
  • sniff: Time: 161ms; Memory: 8MB (Success)
  • stan: [ERROR] Found 188 errors, Failed to run stan admin_toolbar -l 8: exit status 1
  • stylelint: 50 errors found, Failed to run stylelint admin_toolbar: exit status 2
  • test: Tests: 17, Assertions: 394, Errors: 2.

Praise: Everything looks pretty good after fixing the version number in the README!

Copy link
Contributor

@byrond byrond left a comment

Choose a reason for hiding this comment

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

@agentrickard I followed the same steps as @sarahwylie, and all the commands are working as expected (some finding errors, some passing).

Do you think we should update to 10.3.6 and latest contrib versions before merge?

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