Skip to content

Conversation

@sachasayan
Copy link
Contributor

@sachasayan sachasayan commented Apr 30, 2025

Quick comment slop fix. Hopefully fewer of these being fed into context means the LLMs (looking at you, Gemini... 👀) are less eager to add even more.

Regex used:

\/\/[^\S\n]+(?:Add|Remove|Update|Import|Restore).{1,30}(\n)

Plain english: "Look for //, followed by [Add(ed), Remove(d), Update(d)...], followed by up to thirty characters and then a newline.


Important

Remove specific types of comments across multiple files to improve code readability and maintainability.

  • Behavior:
    • Removes comments matching the regex \/\/[^\S\n]+(?:Add|Remove|Update|Import|Restore).{1,30}(\n) across various files.
    • Affects comments that start with // followed by keywords like Add, Remove, Update, etc.
  • Files Affected:
    • gemini.test.ts, mistral.test.ts, deepseek.ts, cache-strategy.test.ts, base-strategy.ts, Cline.ts, Cline.test.ts, ContextProxy.ts, CustomModesManager.ts, insert-groups.ts, multi-search-replace.test.ts, index.ts, system.test.ts.snap, system.test.ts, custom-system-prompt.test.ts, custom-instructions.ts, index.ts, executeCommandTool.test.ts, applyDiffTool.ts, ClineProvider.ts, ClineProvider.test.ts, webviewMessageHandler.ts, DiffViewProvider.ts, processCarriageReturns.benchmark.ts, extract-text.ts, read-lines.ts, ShellIntegrationManager.ts, Terminal.ts, TerminalProcess.ts, TerminalProcess.test.ts, TerminalProcessExec.bash.test.ts, TerminalProcessExec.cmd.test.ts, TerminalProcessExec.pwsh.test.ts, TerminalRegistry.test.ts, bashStream.ts, BrowserSession.ts, browserDiscovery.ts, ShadowCheckpointService.test.ts, McpHub.ts, McpHub.test.ts, file-search.ts, sample-rust.ts, languageParser.test.ts, index.ts, modes.ts, shell.test.ts, CompactTransport.test.ts, BrowserSessionRow.tsx, ChatView.tsx, ChatTextArea.test.tsx, TaskHeader.test.tsx, CodeBlock.tsx, MermaidBlock.tsx, SettingsView.test.tsx, TemperatureControl.test.tsx, select-dropdown.tsx, RooTips.test.tsx, ExtensionStateContext.tsx, command-validation.ts, context-mentions.ts, highlight.ts.
  • Misc:
    • Improves code readability by removing redundant comments.

This description was created by Ellipsis for 9ce967b. You can customize this summary. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Apr 30, 2025

⚠️ No Changeset found

Latest commit: 2c6a39b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 30, 2025
@sachasayan sachasayan marked this pull request as ready for review April 30, 2025 22:02
@sachasayan sachasayan requested a review from mrubens as a code owner April 30, 2025 22:02
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 30, 2025
@ellipsis-dev
Copy link
Contributor

ellipsis-dev bot commented Apr 30, 2025

The changes in this pull request are focused on removing redundant comments and cleaning up code formatting across multiple files. These changes are consistent and related, as they aim to improve code readability and maintainability by removing unnecessary comments. Therefore, it makes sense to keep them in a single pull request for consistency.

@adamhill
Copy link
Contributor

adamhill commented Apr 30, 2025

Some of the Regex removables might be usefully explanatory. (it is regex after all)

So Is there going to be / should be a prohibition against all the trigger words for comments?

Or as a project we agree that "all human written comments" use /// or //<some-character(s)> to protect against future AI slop?

@sachasayan
Copy link
Contributor Author

Some of the Regex removables might be usefully explanatory. (it is regex after all)

I agree. I did limit character length to 30 chars — anything beyond that stayed — and I did a quick manual pass for anything that looked valuable WITHIN the captured lines. This is, granted, otherwise a blunt measure.

Or as a project we agree that "all human written comments" use /// or //<some-character(s)> to protect against future AI slop?

Sooner or later the AI is going to adapt and start feeding triple-slash slop into the codebase, so I don't think that'll work. However, I think this is also going to get fixed in the models within 3-6 months so this is just a temporary sanity measure?

@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap May 1, 2025
@sachasayan
Copy link
Contributor Author

Closing this one. Will re-open after #3235 is merged.

@sachasayan sachasayan closed this May 6, 2025
@github-project-automation github-project-automation bot moved this from PR [Pre Approval Review] to Done in Roo Code Roadmap May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants