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: windows compatibility issues #129

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

Conversation

dustin-jw
Copy link
Contributor

@dustin-jw dustin-jw commented Oct 31, 2023

Description

This addresses some compatibility issues that developers might run into on Windows, either through refactoring or documentation. Notable changes include:

  • More documentation about development setup for Windows users, including setting the shell for npm scripts and calling out possible issues
  • More documentation about VS Code extensions for PHP/Twig and how to set them up for real-time linting
  • Changing the volume mappings to avoid needing to mount to a tmp directory for run.sh commands
  • Favoring docker compose commands in shell scripts to avoid needing to change the container name when customizing the project
  • Excluding a linting rule that would break on Windows with git's autocrlf behavior

Closes #126

To Validate

macOS

For macOS, we want to make sure everything is still working as expected, so this looks like a lot of steps, but the checklist should be fairly quick to run through.

  1. Make sure all PR Checks have passed
  2. Pull down this branch
  3. Stop your development process if it's already running (Ctrl+C, docker compose down)
  4. Work through this checklist to confirm everything works for macOS:
  • Run npm ci
  • If this is your first time setting up the project, copy .env.example to .env (no need to change the default values)
  • Run docker compose build
  • Run npm run lint and confirm that the PHP/Twig linting scripts work as expected (should be no errors found)
  • Run npm run php:run composer install and confirm that it runs without errors or with any changes to the lock file
  • Run npm run php:run composer require wpackagist-plugin/advanced-custom-fields and confirm that both composer.json and composer.lock are updated
  • Run npm start and confirm the local site is working as expected
  • Run npm run export-db and confirm that the DB export creates a .sql.gz file in sql/exports
  • Run npm run backup-db and confirm that the DB backup creates a .sql.gz file in sql/backups
  • Make a change in WordPress that will be easy to verify, like adding, editing, or deleting a post
  • Copy one of the exported/backed up SQL dumps to sql and run npm run import-db
  • In WordPress, confirm that the change you made is reverted (back to the state of the backup/export)
  • Run npm run map-vendor-files and confirm that a top-level vendors file is created and populated with files from the container
  • If using VS Code, install the PHP Sniffer & Beautifier and twigcs extensions and update or create your .vscode/settings.json to match the example in the README
  • Open a PHP file and introduce a linting error (bad indentation, missing space after open parens, etc.) and confirm that you see error highlighting
  • Attempt to format the PHP file via context menu or keyboard shortcut (you may need to specify PHP Sniffer & Beautifier as the formatter) and confirm that it fixes the issue
  • Open a Twig file and introduce a linting error and confirm that you see error highlighting
  • While the container is running, run npm run lint and confirm that it runs as expected (no errors assuming you fixed the ones you introduced for testing)

Windows

On Windows, we want to validate that there aren't any compatibility issues with our development process, and we want to confirm that our documentation is accurate.

  1. Pull down this branch
  2. Work through this checklist to make sure everything works without any special setup steps other than having Node.js, Docker, and git (with git bash) installed:
  • Run npm config set script-shell "C:\\Program Files\\git\\bin\\bash.exe", assuming you have git bash installed (you may need to change a path segment to Program Files (x86) if you're on a 32 bit system)
  • Run npm ci
  • If this is your first time setting up the project, copy .env.example to .env (no need to change the default values)
  • Run docker compose build
  • Run npm run lint and confirm that the PHP/Twig linting scripts work as expected (should be no errors found)
  • Run npm run php:run composer install and confirm that it runs without errors or with any changes to the lock file
  • Run npm run php:run composer require wpackagist-plugin/advanced-custom-fields and confirm that both composer.json and composer.lock are updated
  • Run npm start and confirm the local site is working as expected
  • Run npm run export-db and confirm that the DB export creates a .sql.gz file in sql/exports
  • Run npm run backup-db and confirm that the DB backup creates a .sql.gz file in sql/backups
  • Make a change in WordPress that will be easy to verify, like adding, editing, or deleting a post
  • Copy one of the exported/backed up SQL dumps to sql and run npm run import-db
  • In WordPress, confirm that the change you made is reverted (back to the state of the backup/export)

@dustin-jw dustin-jw force-pushed the fix--windows-compatibility branch from 5d093bf to 5e060e7 Compare October 31, 2023 20:27
@dustin-jw dustin-jw marked this pull request as ready for review October 31, 2023 20:27
@dustin-jw dustin-jw force-pushed the fix--windows-compatibility branch 6 times, most recently from 9a55470 to 741ee44 Compare November 7, 2023 18:09
@dustin-jw dustin-jw force-pushed the fix--windows-compatibility branch from 741ee44 to d77c576 Compare November 9, 2023 20:54
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.

Fix/document Windows compatibility issues
1 participant