-
Notifications
You must be signed in to change notification settings - Fork 1
refactor(oocana): move string_hash to internal.py #459
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?
Conversation
The string_hash utility function was defined in context.py but belongs with other utility functions in internal.py. This improves code organization.
Walkthrough将 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
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.
Pull request overview
This PR refactors the codebase by moving the string_hash() utility function from context.py to internal.py, where other utility functions are centralized. This improves code organization by grouping utility functions together.
Changes:
- Moved
string_hash()function definition fromcontext.pytointernal.py - Updated import in
context.pyto importstring_hashfrominternal - Removed unused
hashlibimport fromcontext.py
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| oocana/oocana/internal.py | Added string_hash() function definition and hashlib import |
| oocana/oocana/context.py | Removed string_hash() definition and hashlib import; updated import statement to include string_hash from internal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def string_hash(text: str) -> str: | ||
| """ | ||
| Generates a deterministic hash for a given string. | ||
| """ | ||
| return hashlib.sha256(text.encode('utf-8')).hexdigest() |
Copilot
AI
Jan 31, 2026
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.
The string_hash function is placed before the InternalAPI class, but the existing random_string utility function is defined after the class (line 40). For consistency, consider moving string_hash to after the InternalAPI class definition, near random_string, to keep all utility functions grouped together.
Summary
string_hash()function fromcontext.pytointernal.pyProblem
string_hash()was defined at module level incontext.pybut is a general utility function that doesn't belong there.Solution
Move to
internal.pywhere other utility functions likerandom_string()are defined.Test Plan