-
Notifications
You must be signed in to change notification settings - Fork 7
DO NOT MERGE testing code reviewer #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -69,6 +69,13 @@ def completion_tokens(self) -> Optional[int]: | |||||
| return v if isinstance(v, int) else None | ||||||
|
|
||||||
|
|
||||||
| class AddingSomethingUseless: | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: this class and the module-level instantiation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class and the module-level instantiation below are unused dead code. The instance There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class is not referenced anywhere else in the repository (only at line 51 in this same file). While the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: this class is unused and appears to be test scaffolding. It should not be committed to the production module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to be dead code. The class and the module-level instantiation below ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code. A class explicitly named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code. This class has no purpose and should not be committed to a production module. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class appears to be dead code with no functional purpose. It should be removed before merging.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class is unused anywhere in the codebase. Dead code adds maintenance burden and should be removed.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: this class is not referenced anywhere and serves no purpose in the codebase. It should be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This class and the instance below are dead code with no functional purpose. Please remove |
||||||
| pass | ||||||
|
|
||||||
|
|
||||||
| p = AddingSomethingUseless() | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The instantiation of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unexplained global state: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code: module-level instantiation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This module-level instance executes at import time, creating an unnecessary side effect and polluting the module namespace. Remove this unused object. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused module-level instantiation of a useless class. This is dead code with no purpose and should be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This module-level instantiation runs at import time, wastes resources, and pollutes the module namespace. Please remove both the class and this line. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Module-level instantiation is an anti-pattern: it triggers side effects on import and leaks a global variable into the module namespace. Please remove. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Module-level instantiation creates an object at import time with no purpose. Remove this dead code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Module-level dead code that instantiates a useless class. This pollutes the module namespace and serves no purpose. Remove before merging.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Module-level instantiation causes a side effect on every import of this module. Even if the class were useful, this should not happen at the top level. Please remove. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Module-level side effect: instantiating an object at import time causes unnecessary work on every module import and pollutes the global namespace. If this is test code, it should not be merged. |
||||||
|
|
||||||
|
|
||||||
| class ChatCompletionClient: | ||||||
| """Minimal OpenAI-compatible /v1/chat/completions client. | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
AddingSomethingUselessclass appears to serve no functional purpose. If it's a placeholder or test, it should be removed or properly documented. Unused code increases maintenance burden.