Skip to content

chore(lint): missing-metadata #311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented Jun 5, 2025

We can make linting faster if we merge the similar llm description and introduced in check into a single pass (rather than the 3-4 passes used prior).

@Copilot Copilot AI review requested due to automatic review settings June 5, 2025 14:34
@avivkeller avivkeller requested a review from a team as a code owner June 5, 2025 14:34
Copy link

codecov bot commented Jun 5, 2025

Codecov Report

Attention: Patch coverage is 96.25000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 49.39%. Comparing base (440da36) to head (87f138f).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/linter/rules/index.mjs 0.00% 2 Missing ⚠️
src/linter/rules/missing-metadata.mjs 98.70% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #311      +/-   ##
==========================================
+ Coverage   48.90%   49.39%   +0.49%     
==========================================
  Files          82       80       -2     
  Lines        6825     6818       -7     
  Branches      277      281       +4     
==========================================
+ Hits         3338     3368      +30     
+ Misses       3484     3447      -37     
  Partials        3        3              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR merges the separate “missing-introduced-in” and “missing-llm-description” rules into one pass (missing-metadata), removes the old utility and rule files, updates tests to use a shared createContext helper, and broadens the ESLint test override glob.

  • Combine metadata checks for introduced_in and llm_description into a single rule to speed up linting.
  • Remove findTopLevelEntry util and two old rules; register the new missing-metadata rule.
  • Add shared test context helper and update tests; adjust ESLint configuration for all __tests__ folders.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/linter/utils/find.mjs Removed redundant findTopLevelEntry util.
src/linter/rules/missing-metadata.mjs Added combined rule with METADATA_CHECKS.
src/linter/rules/missing-llm-description.mjs Deleted old separate llm description rule.
src/linter/rules/missing-introduced-in.mjs Deleted old separate introduced-in rule.
src/linter/rules/index.mjs Swapped in missing-metadata and removed the two rules.
src/linter/rules/tests/utils.mjs Introduced createContext helper for tests.
src/linter/rules/tests/missing-metadata.test.mjs New tests covering combined metadata rule.
eslint.config.mjs Expanded override glob to all __tests__ directories.
Comments suppressed due to low confidence (3)

src/linter/rules/missing-metadata.mjs:7

  • The constant name INTRDOCUED_IN_REGEX appears misspelled. It should be INTRODUCED_IN_REGEX to match its definition and ensure the regex is applied correctly.
  INTRDOCUED_IN_REGEX,

src/linter/rules/tests/utils.mjs:3

  • Add a default path property (e.g. path: 'file.md') to the returned context so that rules expecting context.path do not break during tests.
export const createContext = children => ({

eslint.config.mjs:68

  • [nitpick] The glob **/__tests__/** may disable JSDoc rules for non-source folders. Consider scoping it to your source directory (e.g. src/**/__tests__/**) to avoid unintended overrides.
    files: ['**/__tests__/**'],

@avivkeller
Copy link
Member Author

If 87f138f is off-topic, I can revert it, but I think it's close enough to allow (it's a typo fix)

Copy link
Member

@araujogui araujogui left a comment

Choose a reason for hiding this comment

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

Great PR. The only downside I see is that users can't disable only the introduced-in or llm-description rules.

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants