Skip to content

docs: collapse duplicate README filtering paragraph - #323

Closed
YoungJinJung wants to merge 1 commit into
mainfrom
chore/issue-317-readme-duplicate-filtering-paragraph
Closed

docs: collapse duplicate README filtering paragraph#323
YoungJinJung wants to merge 1 commit into
mainfrom
chore/issue-317-readme-duplicate-filtering-paragraph

Conversation

@YoungJinJung

Copy link
Copy Markdown
Contributor

Summary

README.md carried the service-list filtering paragraph twice in a row (lines 476–477 on dbee17a). This was fallout from the browser PR merges — each merge resolved the paragraph against a different base, and one round left both copies in place.

Neither copy was complete:

first copy second copy
CloudFormation stacks
Auto Scaling groups
DynamoDB tables
EventBridge rules and targets

Changes

  • Collapse to a single paragraph carrying the union of both coverage lists.
  • Add the four filterable screens neither copy listed: security groups, CloudWatch alarms, CloudTrail events, KMS keys.

The additions were verified against the code, not guessed — each has both an activateFilter(<target>) and an updateSharedFilter(msg, <target>) call site, same as every other entry in the list.

Validation

  • go build ./...
  • go test ./... — all packages pass
  • grep -c 'The service list defaults to favorites first' README.md1
  • Scanned the rest of README.md for other duplicated long lines — none

Documentation only; no runtime behaviour changes.

Closes #317

The service-list filtering paragraph was duplicated during the browser PR
merges. Neither copy was complete: the stale one listed CloudFormation
stacks, the newer one listed Auto Scaling groups, DynamoDB tables, and
EventBridge rules and targets.

Keep a single paragraph carrying the union, and add the four filterable
screens neither copy listed (security groups, CloudWatch alarms,
CloudTrail events, KMS keys) so the coverage list matches the
filterTarget constants actually wired to activateFilter.

Closes #317

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P2bi5Xga4wU5hDPgNPd4mv
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 21 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e1eda0c6-a8ab-488a-b826-9657abe42de6

📥 Commits

Reviewing files that changed from the base of the PR and between dbee17a and 732e7ae.

📒 Files selected for processing (1)
  • README.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@amazon-q-developer amazon-q-developer Bot 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 documentation change correctly addresses the duplicate paragraph issue. The consolidated paragraph now includes all filterable screens from both original copies plus the four additional screens (security groups, CloudWatch alarms, CloudTrail events, KMS keys) that were missing from both. The change is accurate, improves documentation quality, and has no runtime impact. Ready to merge.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

@YoungJinJung

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #318, which was opened first and resolves #317 correctly (union of both coverage lists).

The one thing this PR had on top is posted as a suggestion on #318.

@YoungJinJung YoungJinJung left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewed head: 732e7ae

Low

  • README.md:476 — The consolidated “Filtering is currently available…” list is still incomplete. At this head, the EC2 related-resource list uses filterEC2BrowserRelated (internal/app/screen_ec2_browser.go:196 and :208) and the Inspector checklist picker uses filterInspectorChecklistFiles (internal/app/screen_inspector.go:470 and :482); both render the shared filter with inline match highlighting, just like the screens named here. The authoritative coverage list therefore still hides supported filtering from users. Add these screens (for example, “EC2 inventory instances/related resources” and “Inspector checklist files”), or explicitly scope the sentence to exclude nested and picker screens.

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.

docs: remove duplicate README filtering paragraph

1 participant