Skip to content

CHANGE @W-22462048@ apex guru tags update - #489

Merged
nikhil-mittal-165 merged 4 commits into
devfrom
apex-guru-tags
Jul 28, 2026
Merged

CHANGE @W-22462048@ apex guru tags update#489
nikhil-mittal-165 merged 4 commits into
devfrom
apex-guru-tags

Conversation

@nikhil-mittal-165

@nikhil-mittal-165 nikhil-mittal-165 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

On the tag removal: The motivation was to introduce a new tag DevPreviewApexGuru as the sole tag on ApexGuru rules. This is tied to a broader change in how ApexGuru rules participate in rule selection — the intent is that ApexGuru rules should only be invocable via explicit opt-in (engine name, rule name, or the DevPreviewApexGuru tag), not via broad selectors like --rule-selector 4 or --rule-selector Performance. Removing COMMON_TAGS.CATEGORIES.* and COMMON_TAGS.LANGUAGES.APEX from these rules is deliberate: those tags would otherwise make ApexGuru rules selectable by category/language selectors in ways we don't want during the dev preview phase.

@git2gus

git2gus Bot commented Jul 28, 2026

Copy link
Copy Markdown

Git2Gus App is installed but the .git2gus/config.json doesn't have right values. You should add the required configuration.

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

This PR silently removes customer-facing tags from every ApexGuru rule and deletes the tests that guarded that behavior, with no PR description explaining why.

Issues:

  1. No PR description. Per the PR Hygiene guideline, the description must explain the change and match the code. This PR body is completely empty, so there's no rationale for why COMMON_TAGS.CATEGORIES.PERFORMANCE/BEST_PRACTICES and COMMON_TAGS.LANGUAGES.APEX are being stripped from all ~24 rules in apexguru-rules.ts. These tags appear to drive customer-facing rule filtering (e.g. --rule-selector), so removing them is a behavior change, not a cosmetic one.

  2. Tests deleted instead of updated. The second commit ("test case fix") removes the COMMON_TAGS import and the four it/it.each blocks in apexguru-rules.test.ts that specifically asserted every rule retains its category and language tags:

    • '%s should retain its category tag'
    • 'every rule should have a category tag (Performance or BestPractices)'
    • 'every rule should have Apex language tag'

    These are exactly the tests that would fail against commit 1's change. Deleting them removes the regression signal rather than updating the assertions to reflect an intentional new tag contract. If the tag removal is deliberate, the tests should be rewritten to assert the new expected tag set, not deleted outright.

Please add a PR description explaining the motivation for dropping these tags (e.g., is ApexGuru moving away from category/language-based filtering? Is this tied to a broader tagging redesign?), and restore test coverage that asserts the new intended tag contract rather than removing coverage entirely.

@aruntyagiTutu aruntyagiTutu 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 the follow-up — this fully addresses the earlier concerns.

  • PR description now clearly explains the motivation: DevPreviewApexGuru is meant to be an opt-in-only tag so ApexGuru rules aren't reachable via broad selectors (--rule-selector Performance, --rule-selector 4, all) during the dev preview phase.
  • The previously-deleted test coverage wasn't just restored — it was replaced with more targeted coverage: apexguru-rules.test.ts/ApexGuruEngine.test.ts updated to the new tag, plus a new DevPreviewApexGuru rule selection behavior suite in rule-selection.test.ts (backed by a new DevPreviewEnginePlugin stub) that explicitly asserts the opt-in-only selection semantics.
  • rules.ts#matchesRuleSelector now actually implements that semantics (excluding all/severity from selectables for devpreviewapexguru-tagged rules), with an inline comment explaining why — good that the behavior matches what the new tests assert rather than just renaming a constant.

Approving.

@nikhil-mittal-165
nikhil-mittal-165 merged commit 728b283 into dev Jul 28, 2026
9 checks passed
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