Skip to content

Add dev tooling: composer, phpcs, Makefile, CI, .gitignore - #71

Merged
jmnote merged 1 commit into
mainfrom
chore/dev-tooling
Sep 6, 2026
Merged

jmnote merged 1 commit into
mainfrom
chore/dev-tooling

Conversation

@jmnote

@jmnote jmnote commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • composer.json: mediawiki/mediawiki-codesniffer, mediawiki/minus-x, php-parallel-lint as require-dev, wired up via composer test
  • .phpcs.xml: MediaWiki coding-standard ruleset (excluding hack/, tests/, and the FunctionComment doc-required-for-public/private sniffs)
  • Makefile: make checks (test + phpcs) mirrors what CI runs; make test currently runs tests/QuotesTest.php
  • .github/workflows/pull-request.yml replaces contributors.yml: runs composer install + make checks on every PR
  • .gitignore (vendor/, node_modules/, composer.lock, hack/demo/temp/) and .vscode/ editor settings
  • Drops .editorconfig, superseded by phpcs
  • Drops README's auto-generated Contributors table along with contributors.yml, the workflow that kept it updated — left in place it would just be a stale, frozen snapshot from here on

Tests

make checks

Passes: tests/QuotesTest.php (15 assertions) + phpcs/parallel-lint/minus-x clean against the current includes/-namespaced code.

🤖 Generated with Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc720a6a17

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Makefile Outdated
Comment on lines +13 to +17
mathjax: ## Pin the resources/MathJax submodule to a tag, e.g. `make mathjax MATHJAX_VERSION=4.1.3`
hack/mathjax.sh $(MATHJAX_VERSION)

screenshots: ## Screenshot a demo page, e.g. `make screenshots directMath` (no demo = every demo in docs/demos.yaml)
hack/demo/demo.sh screenshot $(filter-out $@,$(MAKECMDGOALS))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the helper scripts before exposing these targets

In the reviewed commit's complete tree, neither hack/mathjax.sh nor hack/demo/demo.sh exists (and the referenced docs/demos.yaml is also absent), so make mathjax and make screenshots both fail immediately with exit 127. Add the missing helper assets or remove these targets until their implementations are included.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed — moved the mathjax/screenshots targets to the follow-up PR that actually adds hack/mathjax.sh, hack/demo/demo.sh, and docs/demos.yaml. This PR's Makefile now only has checks/test/phpcs, so make mathjax/make screenshots correctly fail with "No rule to make target" until those scripts land.

Comment thread Makefile Outdated
Comment on lines +22 to +23
%:
@:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restrict the catch-all rule to valid screenshot arguments

Because % matches every otherwise unknown goal, any typo such as make cheks executes @: and exits successfully without running the intended checks. This can give developers or automation a false indication that validation completed; pass the demo name through a variable or enumerate valid demo goals instead of accepting every target.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch. Since the catch-all target moves to that same follow-up PR along with mathjax/screenshots, I scoped it there: ifneq ($(filter screenshots,$(MAKECMDGOALS)),) guards the %: rule so it only fires when screenshots is actually one of the invoked goals — make cheks still fails with "No rule to make target" instead of silently no-op'ing.

- composer.json: mediawiki-codesniffer, minus-x, php-parallel-lint as
  require-dev, wired up via `composer test`
- .phpcs.xml: MediaWiki coding-standard ruleset (excluding hack/, tests/,
  and the FunctionComment doc-required-for-public/private sniffs)
- Makefile: `make checks` (test + phpcs) mirrors what CI runs;
  `make test` currently runs tests/QuotesTest.php. The mathjax/screenshots
  targets land later, alongside the hack/ scripts they call.
- .github/workflows/pull-request.yml replaces contributors.yml: runs
  `composer install` + `make checks` on every PR
- .gitignore (vendor/, node_modules/, composer.lock, hack/demo/temp/) and
  .vscode/ editor settings; drops .editorconfig, superseded by phpcs
- Drops README's auto-generated Contributors table along with
  contributors.yml, the workflow that kept it updated — left in place it
  would just be a stale, frozen snapshot from here on

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@jmnote
jmnote merged commit 59fc0b8 into main Sep 6, 2026
1 check passed
@jmnote
jmnote deleted the chore/dev-tooling branch September 6, 2026 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant