Skip to content

Run format before format:check in init hydration test validation - #952

Merged
dahlia merged 4 commits into
fedify-dev:mainfrom
Palcimer:issue-950-run-format-before-check
Jul 27, 2026
Merged

Run format before format:check in init hydration test validation#952
dahlia merged 4 commits into
fedify-dev:mainfrom
Palcimer:issue-950-run-format-before-check

Conversation

@Palcimer

@Palcimer Palcimer commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Current validateDevToolScripts only runs format:check and lint after scaffolding a project, without auto-formatting it first. Since neither Fedify's own generated files nor most scaffolding CLI output exactly match the project's chosen formatter's style, format:check almost always failed for npm/pnpm package managers, causing the entire hydration test suite to fail regardless of the actual generated code. Fixed by running format script before format:check so that scaffolded projects are already compliant with the formatter before being checked.

Assisted-by: Claude Code:claude-sonnet-5

Related issue

Changes

  • Added format script before format:check in validateDevToolScripts function.

Benefits

  • Test suites for package managers npm/pnpm will not fail because of format issue.

Checklist

  • Did you add a changelog entry to the CHANGES.md?
  • [] Did you write some relevant docs about this change (if it's a new feature)?
  • [] Did you write a regression test to reproduce the bug (if it's a bug fix)?
  • [] Did you write some tests for this change (if it's a new feature)?
  • Did you run mise test on your machine?

@netlify

netlify Bot commented Jul 18, 2026

Copy link
Copy Markdown

Deploy Preview for fedify-json-schema canceled.

Name Link
🔨 Latest commit 4ca0eab
🔍 Latest deploy log https://app.netlify.com/projects/fedify-json-schema/deploys/6a66e32930840f0008048fbb

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the validateDevToolScripts function in packages/init/src/test/create.ts to include the "format" script in the validation loop alongside "format:check" and "lint". There are no review comments, and I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8f813189-6e6e-4fd8-ab93-341ad25a541d

📥 Commits

Reviewing files that changed from the base of the PR and between 28d1d68 and 4ca0eab.

📒 Files selected for processing (2)
  • CHANGES.md
  • packages/init/src/test/create.ts

📝 Walkthrough

Walkthrough

Hydration test validation now runs format, format:check, and lint for non-Deno package managers, continues after script failures with warnings, and documents the fix in the 2.4.0 changelog.

Changes

Developer-tool validation

Layer / File(s) Summary
Update validation script sequence
packages/init/src/test/create.ts, CHANGES.md
validateDevToolScripts removes Astro-specific gating, runs formatting before format checking and linting, captures outputs, continues after non-zero exits with warnings, and records the change in the changelog.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels: component/lint

Suggested reviewers: dahlia, 2chanhaeng

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The script failure handling was altered to ignore non-zero exits and always continue, which is unrelated to the requested formatting fix. Restore the original failure behavior or split that change into a separate PR if it is intended.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change in the PR.
Description check ✅ Passed The description matches the code changes and explains the formatting validation fix.
Linked Issues check ✅ Passed The PR runs format before format:check during init hydration validation, satisfying issue #950.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/init/src/test/create.ts`:
- Line 158: Update the shared validation loop to include only format:check and
lint, removing format from the scripts iterated there. Preserve the existing
Astro-specific format execution so Astro projects run format once, while other
frameworks are not required to define a format script.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 743a417a-8773-43b8-9f92-3fdd8fe34851

📥 Commits

Reviewing files that changed from the base of the PR and between 2d46cdb and 918033f.

📒 Files selected for processing (1)
  • packages/init/src/test/create.ts

Comment thread packages/init/src/test/create.ts
@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.
see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Palcimer
Palcimer force-pushed the issue-950-run-format-before-check branch from 918033f to 7cd7f85 Compare July 19, 2026 03:11
Comment thread packages/init/src/test/create.ts
Comment thread packages/init/src/test/create.ts
@Palcimer
Palcimer requested a review from 2chanhaeng July 27, 2026 04:21
@2chanhaeng

Copy link
Copy Markdown
Member

Great! Before merging this PR, please rebase from the main branch.

Palcimer added 4 commits July 27, 2026 13:48
`validateDevToolScripts` only ran `format:check` and `lint` after
scaffolding a project, without auto-formatting it first.  Since neither
Fedify's own generated files nor most scaffolding CLI output exactly
match the project's chosen formatter's style, `format:check` almost
always failed for `npm`/`pnpm` package managers, causing the entire
hydration test suite to fail regardless of the actual generated code.

fedify-dev#950

Assisted-by: Claude Code:claude-sonnet-5
`validateDevToolScripts` special-cased Astro to run `format` before
`format:check`/`lint`, added in 5f10801 while fixing Astro 5-7
compatibility.  Since the previous commit now runs `format` for every
framework before `format:check`, this block just ran `format` a second
time for Astro with no effect, so it's dead code now — removed it.

fedify-dev#950

Assisted-by: Claude Code:claude-sonnet-5
`validateDevToolScripts()` returned `false` when a script failed,
which kept the directory out of the later lookup test even when the
scaffolded app itself worked fine.  These scripts aren't essential to
whether the app runs, so print a warning instead of returning
`false`, letting the lookup test proceed regardless.

fedify-dev#950

Assisted-by: Claude Code:claude-sonnet-5
@Palcimer
Palcimer force-pushed the issue-950-run-format-before-check branch from 28d1d68 to 4ca0eab Compare July 27, 2026 04:48
@Palcimer
Palcimer requested a review from 2chanhaeng July 27, 2026 07:14
@dahlia
dahlia merged commit 1eadcb0 into fedify-dev:main Jul 27, 2026
24 checks passed
@dahlia dahlia added the component/cli CLI tools related label Jul 27, 2026
@dahlia dahlia added this to the Fedify 2.4 milestone Jul 27, 2026
@dahlia dahlia self-assigned this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/cli CLI tools related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run format before format:check in fedify init's hydration test validation

3 participants