Skip to content

Remove annotations from related files regardless of exclude_* options#343

Open
jeduardo824 wants to merge 1 commit into
drwl:mainfrom
jeduardo824:fix/delete-ignores-excluded-related-files
Open

Remove annotations from related files regardless of exclude_* options#343
jeduardo824 wants to merge 1 commit into
drwl:mainfrom
jeduardo824:fix/delete-ignores-excluded-related-files

Conversation

@jeduardo824

Copy link
Copy Markdown

When a related file type is excluded (e.g. exclude_tests: true), annotaterb models --delete skipped those files, leaving previously written annotations orphaned with no built-in way to clean them up.

ProjectAnnotationRemover reuses RelatedFilesListBuilder, which gates each related type on the exclude_* options for both annotation and removal. Add a for_removal: flag (default false) that bypasses those guards so removal reaches every related file; the annotate path is unchanged. Removing an annotation from a file that has none is a no-op.

Refs #340

When a related file type is excluded (e.g. `exclude_tests: true`),
`annotaterb models --delete` skipped those files, leaving previously
written annotations orphaned with no built-in way to clean them up.

ProjectAnnotationRemover reuses RelatedFilesListBuilder, which gates each
related type on the `exclude_*` options for both annotation and removal.
Add a `for_removal:` flag (default false) that bypasses those guards so
removal reaches every related file; the annotate path is unchanged.
Removing an annotation from a file that has none is a no-op.

Refs drwl#340

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@OdenTakashi OdenTakashi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for fixing this. Looks great to me!

However, as I mentioned in issue #340, there is a possibility that a certain usage pattern may no longer work after this change, so I’d like @drwl to take a look at that point.

That said, I don’t think that usage pattern is actually intended, so I’m personally fine with merging this as-is.

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