Skip to content

Follow up on #32, minimize future diffs#110

Merged
mbreiser merged 3 commits into
reiserlab:mainfrom
floesche:formatting/run_formatter_again
Jun 16, 2026
Merged

Follow up on #32, minimize future diffs#110
mbreiser merged 3 commits into
reiserlab:mainfrom
floesche:formatting/run_formatter_again

Conversation

@floesche

Copy link
Copy Markdown
Member

Follow up on #32. Having this in main and then using the .prettierrc as the definition of what the code looks like, increases readability of code and code changes.

@floesche
floesche requested a review from mbreiser June 15, 2026 17:22
@floesche floesche mentioned this pull request Jun 15, 2026

@mbreiser mbreiser left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this, Frank — agreed that having main match .prettierrc makes diffs cleaner, and I'd like to land it. One ask before we merge:

71 of the 100 files are third-party vendored dependencies, not our code:

  • js/vendor/yaml/** (70 files) — the yaml npm package (Eemeli Aro, ISC); our checked-in copy that experiment_designer_v3.html imports.
  • js/gif.worker.js — jnordberg's gif.js worker (prettier un-minified it from 3 lines → ~886).

Reformatting these actually works against "minimize future diffs": once we restyle vendored yaml, every future version bump (2.9 → 2.10 …) diffs against our reformatted copy instead of upstream, burying real changes in style noise. And un-minifying the gif worker just bloats it for no benefit.

Could you add a .prettierignore and re-run npm run format?

# Vendored / third-party — keep byte-identical to upstream
js/vendor/
js/gif.worker.js

That drops the PR to ~29 first-party files (the part worth reviewing), and stops npm run format from churning vendored code going forward (it currently globs **/*.js, which is why this happened). The first-party-only reformat looks good to me — happy to merge as soon as it's rescoped. Thanks!

@floesche

Copy link
Copy Markdown
Member Author

I moved the gif.worker.js file to vendor, but otherwise followed the suggestion.

@mbreiser
mbreiser merged commit 447fb5d into reiserlab:main Jun 16, 2026
4 checks passed
mbreiser pushed a commit that referenced this pull request Jun 16, 2026
…gen (LAB-111 follow-up)

js/arena-configs.js is generated by scripts/generate-arena-configs.js
(JSON.stringify, 2-space). PR #110 ran Prettier over it (4-space), so the
generator and Prettier disagreed on every sync run — each sync then produced a
~180-line all-formatting diff (e.g. the bot's PR #111).

- .prettierignore: add js/arena-configs.js (generated, same principle as js/vendor/).
- regenerate to canonical generator output (reverts #110's reformat of this one
  file). Config data unchanged; ARENA_REGISTRY intact; npm test green.

Future syncs now diff only on real config changes (plus the timestamp).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@floesche
floesche deleted the formatting/run_formatter_again branch June 30, 2026 17:33
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.

2 participants