-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ref(derived_code_mappings): Simplify FrameFilename #85981
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
Conversation
Changes included: * Remove some unused properties * Unify code that transforms the file path
from sentry.integrations.manager import default_manager as integrations | ||
from sentry.integrations.services.integration import RpcIntegration, integration_service | ||
from sentry.issues.auto_source_code_config.code_mapping import FrameFilename, find_roots | ||
from sentry.issues.auto_source_code_config.code_mapping import FrameInfo, find_roots |
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.
I'm renaming the class as it does not just hold a modified frame filename.
frame_file_path = frame_file_path.replace("\\", "/") | ||
|
||
if frame_file_path[0] == "/" or frame_file_path[0] == "\\": | ||
frame_file_path = frame_file_path[1:] |
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.
raise UnsupportedFrameFilename("This path is not supported.") | ||
raise UnsupportedFrameInfo("This path is not supported.") | ||
|
||
self.full_path = frame_file_path |
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.
Removing it as it's only used within this module:
https://github.com/search?q=repo%3Agetsentry%2Fsentry+.full_path+path%3A%22src%2Fsentry%2Fissues%2F%22&type=code
raise UnsupportedFrameInfo("This path is not supported.") | ||
|
||
self.full_path = frame_file_path | ||
self.extension = get_extension(frame_file_path) |
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.
Same as the above.
# windows drive letters can be like C:\ or C: | ||
# so we need to remove the slash if it exists | ||
if frame_file_path[0] == "/": | ||
frame_file_path = frame_file_path[1:] |
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.
frame_file_path = frame_file_path[1:] | ||
|
||
start_at_index = get_straight_path_prefix_end_index(frame_file_path) | ||
self.straight_path_prefix = frame_file_path[:start_at_index] |
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.
Not in use.
start_at_index = get_straight_path_prefix_end_index(frame_file_path) | ||
if start_at_index == 0: | ||
self.root = frame_file_path.split("/")[0] | ||
self.stack_root = frame_file_path.split("/")[0] |
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.
Renaming it to something more meaningful.
self.root = frame_file_path[0:slash_index] | ||
self.stack_root = frame_file_path[0:slash_index] | ||
|
||
def transformations(self, frame_file_path: str) -> str: |
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.
Copy pasted unchanged from two blocks in the __init__
function.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Changes included: * Remove some unused properties * Unify code that transforms the file path
Changes included: