External/card-drop
Watch
5
0
Fork
You've already forked card-drop
 0
Code Issues 60 Pull requests Projects 1 Releases 37 Packages Wiki Activity Actions

Finish repo-wide strict equality sweep and enable eqeqeq #517

New issue
Closed
opened 2026-09-03 12:05:54 +01:00 by Smithy-bot · 7 comments
Smithy-bot commented 2026-09-03 12:05:54 +01:00
Member
Copy link

Parent: #487 (2026 Code Clean Up). Spun out of the review note on PR #515.

Why

PR #515 only converted the comparisons in the five files named by the #487 checklist (InventoryHelper, GetUnclaimedCardsHelper, stats, GetCardsHelper, sacrifice) — 20 ==/!= sites.

At the time of that PR there were still about 73 loose comparisons across 37 files. eqeqeq was not enabled in ESLint, because turning it on fails lint on every unswept file; it has to land with (or after) the final sweep.

This is mechanical and is much faster with a local yarn lint:fix than through the bot's one-file-at-a-time API.

Suggested approach

  1. Merge or rebase on top of #515 if it is not already on release/0.12.0.
  2. Enable eqeqeq in eslint.config.mjs (error).
  3. Run yarn lint:fix (or equivalent) and clear every remaining ==/!= the rule reports.
  4. Pay attention to intentional null checks — prefer == null only where you truly want both null and undefined, or rewrite to explicit checks; do not leave bare ==.
  5. Run yarn build, yarn lint, yarn test.
  6. Especially re-test paths that compare card fields, quantities, and timer IDs. If card metadata validation (#516) is not done yet, exercise /drop, /inventory, and /stats against real cards before release.

Acceptance criteria

  • No remaining == / != in src/ / tests/ except any explicitly justified, lint-disabled sites.
  • eqeqeq is enabled and yarn lint is clean.
  • yarn build and yarn test are clean.
  • Brief note in the PR if any comparison was intentionally left as loose (with a disable comment).

Related

  • PR #515 — partial sweep of the checklist files
  • Sibling #516 — validate card metadata types at parse time (prefer before relying on === against real card JSON)
  • Parent #487

Note on eslint config globs

Separately, eslint.config.mjs currently scopes a rules block to files: ["./src", "./tests"], which matches no files, so several style rules are inert. That is a different fix; do not conflate it with enabling eqeqeq unless you are already touching that config.

Parent: #487 (2026 Code Clean Up). Spun out of the review note on PR #515. ## Why PR #515 only converted the comparisons in the five files named by the #487 checklist (`InventoryHelper`, `GetUnclaimedCardsHelper`, `stats`, `GetCardsHelper`, `sacrifice`) — **20** `==`/`!=` sites. At the time of that PR there were still about **73** loose comparisons across **37** files. `eqeqeq` was **not** enabled in ESLint, because turning it on fails lint on every unswept file; it has to land with (or after) the final sweep. This is mechanical and is much faster with a local `yarn lint:fix` than through the bot's one-file-at-a-time API. ## Suggested approach 1. Merge or rebase on top of #515 if it is not already on `release/0.12.0`. 2. Enable `eqeqeq` in `eslint.config.mjs` (error). 3. Run `yarn lint:fix` (or equivalent) and clear every remaining `==`/`!=` the rule reports. 4. Pay attention to intentional null checks — prefer `== null` only where you truly want both `null` and `undefined`, or rewrite to explicit checks; do not leave bare `==`. 5. Run `yarn build`, `yarn lint`, `yarn test`. 6. Especially re-test paths that compare card fields, quantities, and timer IDs. If card metadata validation (#516) is not done yet, exercise `/drop`, `/inventory`, and `/stats` against real cards before release. ## Acceptance criteria - [ ] No remaining `==` / `!=` in `src/` / `tests/` except any explicitly justified, lint-disabled sites. - [ ] `eqeqeq` is enabled and `yarn lint` is clean. - [ ] `yarn build` and `yarn test` are clean. - [ ] Brief note in the PR if any comparison was intentionally left as loose (with a disable comment). ## Related - PR #515 — partial sweep of the checklist files - Sibling #516 — validate card metadata types at parse time (prefer before relying on `===` against real card JSON) - Parent #487 ## Note on eslint config globs Separately, `eslint.config.mjs` currently scopes a `rules` block to `files: ["./src", "./tests"]`, which matches **no** files, so several style rules are inert. That is a different fix; do not conflate it with enabling `eqeqeq` unless you are already touching that config.
Smithy-bot added this to the 0.12.0 milestone 2026-09-03 12:05:54 +01:00
Vylpes commented 2026-09-06 09:27:40 +01:00
Owner
Copy link

Blocked until #516 is done

Blocked until #516 is done
Vylpes added spent time 2026-09-07 12:32:19 +01:00
30 minutes
Vylpes removed their assignment 2026-09-08 14:30:11 +01:00
Smithy-bot commented 2026-09-10 12:07:17 +01:00
Author
Member
Copy link

QA results (step/testing) — FAIL

Tested on release/0.12.0 @ d4bd48c (includes merged #519 strict-equality sweep and #521 eslint files-glob fix).

Check Result
eqeqeq enabled (eslint.config.mjs, always + null: ignore) PASS
Remaining == / != in src/ / tests/ PASS — only != null / == null-style sites left (allowed by null: ignore); no bare loose equality
yarn build (tsc) PASS
yarn test PASS — 28 suites / 196 tests / 8 snapshots
yarn lint FAIL

Lint failure

yarn lint reports 8× comma-dangle in tests/stringDropdowns/Inventory.test.ts (lines 28, 29, 36, 44, 46, 47, 53, 93). All auto-fixable with eslint --fix.

These only surface now because #521 corrected the eslint files glob so style rules actually apply to tests/**/*. No eqeqeq violations.

Next step

Moving back to step/todo for a small follow-up: strip the trailing commas (or yarn lint:fix that file), confirm yarn lint is clean, then re-enter review/testing.

Manual Discord smoke (/drop, /inventory, /stats) still recommended at UAT once lint is green — not runnable here without DATA_DIR card assets.

## QA results (step/testing) — FAIL Tested on `release/0.12.0` @ `d4bd48c` (includes merged #519 strict-equality sweep and #521 eslint files-glob fix). | Check | Result | | --- | --- | | `eqeqeq` enabled (`eslint.config.mjs`, `always` + `null: ignore`) | PASS | | Remaining `==` / `!=` in `src/` / `tests/` | PASS — only `!= null` / `== null`-style sites left (allowed by `null: ignore`); no bare loose equality | | `yarn build` (`tsc`) | PASS | | `yarn test` | PASS — 28 suites / 196 tests / 8 snapshots | | `yarn lint` | **FAIL** | ### Lint failure `yarn lint` reports **8× `comma-dangle`** in `tests/stringDropdowns/Inventory.test.ts` (lines 28, 29, 36, 44, 46, 47, 53, 93). All auto-fixable with `eslint --fix`. These only surface now because #521 corrected the eslint `files` glob so style rules actually apply to `tests/**/*`. No `eqeqeq` violations. ### Next step Moving back to `step/todo` for a small follow-up: strip the trailing commas (or `yarn lint:fix` that file), confirm `yarn lint` is clean, then re-enter review/testing. Manual Discord smoke (`/drop`, `/inventory`, `/stats`) still recommended at UAT once lint is green — not runnable here without `DATA_DIR` card assets.
Smithy-bot removed their assignment 2026-09-11 12:05:36 +01:00
Smithy-bot commented 2026-09-11 12:07:01 +01:00
Author
Member
Copy link

Lint follow-up ready for review

Opened #524 off release/0.12.0 (cursor/517-inventory-test-comma-dangle).

Fixes the 8× comma-dangle failures in tests/stringDropdowns/Inventory.test.ts from the previous QA run. Locally: yarn lint, yarn build, and yarn test (28/196) all pass.

Moved to step/review and assigned to @Vylpes. Please approve/merge when happy; do not auto-close this ticket from the PR (needs testing after merge).

## Lint follow-up ready for review Opened https://git.vylpes.xyz/External/card-drop/pulls/524 off `release/0.12.0` (`cursor/517-inventory-test-comma-dangle`). Fixes the 8× `comma-dangle` failures in `tests/stringDropdowns/Inventory.test.ts` from the previous QA run. Locally: `yarn lint`, `yarn build`, and `yarn test` (28/196) all pass. Moved to `step/review` and assigned to @Vylpes. Please approve/merge when happy; do not auto-close this ticket from the PR (needs testing after merge).
Vylpes removed their assignment 2026-09-11 14:02:33 +01:00
Smithy-bot removed their assignment 2026-09-12 12:07:03 +01:00
Smithy-bot commented 2026-09-12 12:07:03 +01:00
Author
Member
Copy link

QA results (step/testing) — PASS

Tested on release/0.12.0 @ 42260e9 (includes merged PR #524 comma-dangle fix for Inventory.test.ts, plus earlier #519/#521).

Check Result
eqeqeq enabled (eslint.config.mjs, always + null: ignore) PASS
Remaining == / != in src/ / tests/ PASS — only 3× != null sites left (allowed by null: ignore); no bare loose equality
yarn build (tsc) PASS
yarn lint (eslint .) PASS
yarn test PASS — 28 suites / 196 tests / 8 snapshots
tests/stringDropdowns/Inventory.test.ts comma-dangle PASS — clean under eslint

Moved step/testing → step/uat and assigned to @Vylpes for UAT.

## QA results (step/testing) — PASS Tested on `release/0.12.0` @ `42260e9` (includes merged PR #524 comma-dangle fix for `Inventory.test.ts`, plus earlier #519/#521). | Check | Result | | --- | --- | | `eqeqeq` enabled (`eslint.config.mjs`, `always` + `null: ignore`) | PASS | | Remaining `==` / `!=` in `src/` / `tests/` | PASS — only 3× `!= null` sites left (allowed by `null: ignore`); no bare loose equality | | `yarn build` (`tsc`) | PASS | | `yarn lint` (`eslint .`) | PASS | | `yarn test` | PASS — 28 suites / 196 tests / 8 snapshots | | `tests/stringDropdowns/Inventory.test.ts` comma-dangle | PASS — clean under eslint | Moved `step/testing` → `step/uat` and assigned to @Vylpes for UAT.
Vylpes commented 2026-09-12 12:14:06 +01:00
Owner
Copy link

I need to consolidate the rules back now

I need to consolidate the rules back now
Vylpes removed their assignment 2026-09-12 12:17:01 +01:00
Smithy-bot removed their assignment 2026-09-13 12:13:09 +01:00
Smithy-bot commented 2026-09-13 12:13:09 +01:00
Author
Member
Copy link

PR #525 first-pass review done (approved).

Your move of eqeqeq into the main rules block is safe: src/**/* / tests/**/* match real files, yarn lint is clean, and a probe == still fails the rule. Requested second review from VylpesTester (you are the author).

Leaving at step/review and assigning to you for merge/second review.

**PR #525 first-pass review done (approved).** Your move of `eqeqeq` into the main rules block is safe: `src/**/*` / `tests/**/*` match real files, `yarn lint` is clean, and a probe `==` still fails the rule. Requested second review from VylpesTester (you are the author). Leaving at `step/review` and assigning to you for merge/second review.
Vylpes removed their assignment 2026-09-13 13:06:02 +01:00
Smithy-bot commented 2026-09-14 12:04:51 +01:00
Author
Member
Copy link

QA results (step/testing) — PASS

Re-tested on release/0.12.0 @ b948124 after merged PR #525 (move eqeqeq into the main src/**/* / tests/**/* rules block).

Check Result
eqeqeq in main rules block (always + null: ignore) PASS
Remaining == / != in src/ / tests/ PASS — only 3× != null sites left (allowed); no bare loose equality
Probe == still fails eqeqeq under the consolidated config PASS
yarn build (tsc) PASS
yarn lint (eslint .) PASS
yarn test PASS — 28 suites / 196 tests / 8 snapshots

Moved step/testing → step/uat and assigned to @Vylpes for UAT.

Manual Discord smoke (/drop, /inventory, /stats) still recommended at UAT — not runnable here without DATA_DIR card assets.

## QA results (step/testing) — PASS Re-tested on `release/0.12.0` @ `b948124` after merged PR #525 (move `eqeqeq` into the main `src/**/*` / `tests/**/*` rules block). | Check | Result | | --- | --- | | `eqeqeq` in main rules block (`always` + `null: ignore`) | PASS | | Remaining `==` / `!=` in `src/` / `tests/` | PASS — only 3× `!= null` sites left (allowed); no bare loose equality | | Probe `==` still fails `eqeqeq` under the consolidated config | PASS | | `yarn build` (`tsc`) | PASS | | `yarn lint` (`eslint .`) | PASS | | `yarn test` | PASS — 28 suites / 196 tests / 8 snapshots | Moved `step/testing` → `step/uat` and assigned to @Vylpes for UAT. Manual Discord smoke (`/drop`, `/inventory`, `/stats`) still recommended at UAT — not runnable here without `DATA_DIR` card assets.
Smithy-bot removed their assignment 2026-09-14 12:04:51 +01:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
release/0.11.0
release/0.12.0
develop
v0.10.1
v0.10.0
v0.9.4
v0.9.3
v0.9.2
v0.9.1
v0.9.0
v0.8.4
v0.8.3
v0.8.2
v0.8.1
v0.8.0
v0.7.0
v0.6.4
v0.6.3
v0.6.2
v0.6.1
v0.6.0
v0.5.2
v0.5.1
v0.5.0
v0.4.2
v0.4.1
v0.4.0
v0.3.1
v0.3.0
v0.2.1
v0.2.0
v0.1.8
v0.1.7
v0.1.6
v0.1.5
v0.1.4
v0.1.3
v0.1.2
v0.1.1
v0.1.0
Labels
Clear labels   
question
More information is needed

  
blocked

   
duplicate
This issue or pull request already exists

  
needs
approval

  
needs
criteria

  
needs
estimate

  
needs
tests

  
question
More information is needed

  
step
doing

  
step
review

  
step
testing

  
step
todo

  
step
uat

  
type
admin

  
type
alert

  
type
bug

  
type
change

  
type
defect

  
type
dependencies

  
type
epic

  
type
idea

  
type
incident

  
type
investigation

  
type
spike

  
type
story

  
wontfix
This won't be fixed

No labels
question
blocked
duplicate
needs
approval
needs
criteria
needs
estimate
needs
tests
question
step
doing
step
review
step
testing
step
todo
step
uat
type
admin
type
alert
type
bug
type
change
type
defect
type
dependencies
type
epic
type
idea
type
incident
type
investigation
type
spike
type
story
wontfix
Milestone
Clear milestone
No items
No milestone
0.12.0
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
Vylpes
2 participants
Notifications
Total time spent: 30 minutes
Vylpes
30 minutes
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
External/card-drop#517
No description provided.