-
Notifications
You must be signed in to change notification settings - Fork 10
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #228 +/- ##
==========================================
- Coverage 89.53% 89.48% -0.06%
==========================================
Files 326 324 -2
Lines 10437 10375 -62
Branches 1908 1904 -4
==========================================
- Hits 9345 9284 -61
+ Misses 1023 1020 -3
- Partials 69 71 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
] | ||
|
||
operations = [ | ||
migrations.AddField( |
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.
let's use the RiskyAddField for this since its updating the repos table
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.
interesting, anything we add to repo is a risky add?
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.
Generally yes (similar with other widely used tables like owners and commits) because the migration will timeout because it will be blocked for a long time by other operations touching the table, but if your new column is (default=False, null=True)
then in theory it should be ok. But I'm not sure that's always the case and past migrations have gotten me paranoid. So IMO just be safe and do risky.
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.
Just in case, the "new" way we get these into prod is by merging this code and then updating the shared version in api/worker. When you do so, schedule sometime with the infra team to run the migration after hours, cause if you merge the code itself, the migrations won't be applied, so they need to run the SQL for you then.
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.
Thank you both!
5ba1dea
Needed to conditionally render onboarding for failed tests.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.