Move the tests/dependencies Linguist rule into the root .gitattributes - #8697
Conversation
context7.json configures the external Context7 documentation indexer (added on purpose in #8417). Nothing in this repository reads it. Deleting it may stop or change Context7's indexing of this repository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
The file marks the vendored @rescript/react copy as linguist-vendored. Only GitHub's language statistics read it; nothing in this repository does. Without it, GitHub may count the vendored ReScript sources in the repository's language breakdown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 883bf2c937
ℹ️ 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".
| @@ -1,49 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
For Context7 consumers, removing this file discards the repository configuration that associates the index with /rescript-lang/rescript, restricts indexed input to the documentation/runtime/tools, and redirects broader language questions to llms.txt. A subsequent external re-index can therefore ingest unrelated compiler and test internals or stop using the intended project configuration, degrading generated answers in a way no repository test detects; retain the file unless the Context7 integration has been explicitly decommissioned.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed. The deletion is reverted in 4039c80, so context7.json and its indexing configuration stay.
| @@ -1,49 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
If removing the Context7 configuration is intentional, it is a user-facing documentation change because it alters how ReScript material is indexed and served to Context7 users, but this diff contains no entry in the current (Unreleased) section. Add the required changelog entry with the PR link so downstream users can discover the change.
AGENTS.md reference: AGENTS.md:L93-L95
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No longer applies: context7.json is kept (4039c80), and removing tests/dependencies/.gitattributes only affects GitHub's language statistics, so there is no user-facing change to record.
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8697 |
|
Not a Context7 user myself, not sure if that is ok to remove. @jderochervlk added it at the time. |
The Context7 indexer still reads it to scope and configure the project's index, so the file stays until that integration is retired. This reverts commit fa762dc. Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cristianoc
changed the title
|
Thanks, agreed: since the Context7 indexer still reads |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8697 +/- ##
==========================================
+ Coverage 78.67% 79.86% +1.19%
==========================================
Files 476 470 -6
Lines 64402 63443 -959
==========================================
+ Hits 50670 50671 +1
+ Misses 13732 12772 -960 🚀 New features to boost your workflow:
|
|
I'd keep this. The fact that nothing in the repository reads it doesn't make it unused—GitHub Linguist is its intended consumer. The rule correctly marks the copied test dependencies as vendored code, excluding them from the repository's language statistics. It doesn't hide their diffs or affect builds, and maintaining this one line costs very little. If the goal is to consolidate configuration, we could move the equivalent rule into the root
Otherwise, I don't see a practical benefit to removing it. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
cristianoc
changed the title
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Cristiano Calcagno <cristianoc@users.noreply.github.com>
|
Agreed, thanks. I moved the rule into the root |
2 participants
Part of #8712.
Summary
tests/dependenciesLinguist rule into the root.gitattributes.tests/dependencies/** linguist-vendoredreplacestests/dependencies/.gitattributes, so the vendored test dependencies stay out of GitHub's language statistics and all attributes live in one file..gitattributesin CONTRIBUTING.md, under "Cloning the Git Repo": LF line endings and the vendored paths.git check-attr linguist-vendoredreportssetfor files undertests/dependencies/.Test plan
make testmake checkformatgit check-attr linguist-vendored -- tests/dependencies/rescript-react/src/React.res🤖 Generated with Claude Code