Skip to content

Fix Windows compatibility: rename files with reserved chars, split large files, add pre-commit hook and CI validation#4239

Open
Copilot wants to merge 7 commits intomasterfrom
copilot/rename-windows-incompatible-files
Open

Fix Windows compatibility: rename files with reserved chars, split large files, add pre-commit hook and CI validation#4239
Copilot wants to merge 7 commits intomasterfrom
copilot/rename-windows-incompatible-files

Conversation

Copy link
Contributor

Copilot AI commented Feb 3, 2026 •
edited by andimarek
Loading

Summary

The repo can't be cloned on Windows due to colons (:) in filenames under performance-results/, and large-schema-5.graphqls exceeds 10 MB.

Changes

  • Rename 242 files in performance-results/ — replace colons in ISO timestamps with hyphens (e.g. T08:40:24Z → T08-40-24Z)
  • Split large-schema-5.graphqls (11.3 MB) into .part1 (7.1 MB) and .part2 (5.1 MB) at a type boundary so each part contains only complete GraphQL type definitions
  • Add pre-commit hook (.githooks/pre-commit) that rejects Windows-incompatible characters and files > 10 MB
  • Add CI workflow (.github/workflows/validate-files.yml) enforcing the same checks on PRs
  • Add scripts/setup-hooks.sh for one-command hook installation
  • Update CONTRIBUTING.md with file validation section

Setup

./scripts/setup-hooks.sh

Copilot AI and others added 2 commits February 3, 2026 19:44
@andimarek
Rename Windows-incompatible files, split large file, and add pre-comm…
052d59d 
…it hook

Co-authored-by: andimarek <1706744+andimarek@users.noreply.github.com>
@andimarek
Improve pre-commit hook: remove set -e, add error handling, use awk i…
21c0f35 
…nstead of bc

Co-authored-by: andimarek <1706744+andimarek@users.noreply.github.com>
Copilot AI changed the title [WIP] Rename Windows-incompatible filenames for compatibility Fix Windows compatibility: rename files with reserved chars, split large files, add pre-commit hook Feb 3, 2026
Copilot AI requested a review from andimarek February 3, 2026 19:49
@andimarek
Copy link
Member

andimarek commented Feb 3, 2026

@copilot also add a github action that checks for the same both conditions, in case something was committed that violates both rules. Can you reuse the commit hook? Is that a good idea?

@andimarek
Add GitHub Action to validate Windows compatibility and file sizes
10098c1 
Co-authored-by: andimarek <1706744+andimarek@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 3, 2026

@copilot also add a github action that checks for the same both conditions, in case something was committed that violates both rules. Can you reuse the commit hook? Is that a good idea?

Added GitHub Action in commit 10098c1. The workflow reuses the same validation logic as the pre-commit hook:

  • Validates Windows-incompatible filenames: Checks all tracked files for reserved characters (< > : " | ? * )
  • Validates file sizes: Ensures no files exceed 10MB
  • Runs on all PRs and pushes: Provides a safety net if local hooks are bypassed
  • Uses GitHub annotations: Highlights specific problematic files in the workflow output

The logic is intentionally similar to the pre-commit hook to ensure consistency, though implemented directly in the workflow YAML rather than calling the hook script. This approach keeps the CI check independent and allows for GitHub-specific features like annotations.

Copilot AI changed the title Fix Windows compatibility: rename files with reserved chars, split large files, add pre-commit hook Fix Windows compatibility: rename files with reserved chars, split large files, add pre-commit hook and CI validation Feb 3, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 3, 2026 •
edited
Loading

Test Results

  335 files  ±0    335 suites  ±0   5m 5s ⏱️ +2s
5 371 tests ±0  5 363 ✅ +1  8 💤  - 1  0 ❌ ±0 
5 460 runs  ±0  5 452 ✅ +1  8 💤  - 1  0 ❌ ±0 

Results for commit 12cc43a. ± Comparison against base commit 8e1c98a.

This pull request removes 196 and adds 172 tests. Note that renamed tests count towards both. 
	?

	, expected: combo-\"\\\b\f\n\r\t, #4]
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
…

graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure23@5f631ca0 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure24@25d0cb3a delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure25@2f4b98f6 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure20@19e0dffe delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure21@4833eff3 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure22@793d163b delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure3@4832f03b delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure4@60c73e58 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure5@1391af3b delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of error args with non null does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_6prov0_closure6@36aa52d2 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
…

♻️ This comment has been updated with latest results.

@andimarek andimarek marked this pull request as ready for review February 11, 2026 21:21
andimarek and others added 2 commits February 12, 2026 07:30
@andimarek @claude
Fix schema split to respect type boundaries, remove LFS references
1123cca 
- Re-split large-schema-5 at a type boundary so each part contains
  only complete GraphQL type definitions
- Remove Git LFS suggestions from pre-commit hook, CI workflow,
  CONTRIBUTING.md, and README
- Simplify large-schema-5-README.md

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@andimarek @claude
Document reason for 10 MB file size limit
12cc43a 
Enterprise users often mirror graphql-java into internal repos
that enforce file size restrictions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dondonz dondonz dondonz approved these changes

@andimarek andimarek Awaiting requested review from andimarek

Assignees

@andimarek andimarek

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@andimarek @dondonz