Skip to content
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

feat(processing): add electron symbol server to default for electron projects #81118

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

shellmayr
Copy link
Member

@shellmayr shellmayr commented Nov 21, 2024

For projects with platforms electron and javascript-electron, add the public electron symbol server to the list of default symbol servers during project creation.

Closes #80891

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 21, 2024
@shellmayr shellmayr changed the title add electron to list and adapt tests feat(processing): add electron symbol server to default for electron projects Nov 21, 2024
Comment on lines 135 to 139
# Add electron symbol server by default to both electron and javascript-electron projects
if project.platform and project.platform in ["electron", "javascript-electron"]:
project.update_option(
"sentry:builtin_symbol_sources", ["ios", "microsoft", "electron"]
)
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the project_created signal is intentionally sent at the very end just before returning the project. Could you reorder this accordingly?

Comment on lines 225 to 230
# Add electron symbol server by default to both electron and javascript-electron projects
if project.platform and project.platform in ["electron", "javascript-electron"]:
project.update_option(
"sentry:builtin_symbol_sources", ["ios", "microsoft", "electron"]
)

Copy link
Member

Choose a reason for hiding this comment

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

This does a very similar thing as set_default_inbound_filters above. Two suggestions

  • Move it up so both of these are placed together. Going forward, this is a good place to add new, similar functionality.
  • Create a helper function that you can also call from the DatabaseBackedProjectService.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I was pondering that, but then we set the seer project option sentry:similarity_backfill_completed below, so I followed that lead. But good points, rearranged and pulled it into a helper function.

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #81118      +/-   ##
==========================================
- Coverage   78.48%   78.47%   -0.01%     
==========================================
  Files        7215     7216       +1     
  Lines      319848   319857       +9     
  Branches    44058    44060       +2     
==========================================
- Hits       251019   251017       -2     
- Misses      62438    62445       +7     
- Partials     6391     6395       +4     

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically enable built-in repos
2 participants