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.

Sign up for GitHub

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

Jump to bottom

Limit error interface #1122

Merged
merged 31 commits into from
Nov 4, 2024
Merged

Limit error interface #1122

merged 31 commits into from
Nov 4, 2024

Conversation

svevang
Copy link
Member

@svevang svevang commented Oct 9, 2024 •
edited
Loading

For #1016

Review #1116 before reviewing this PR

  • Limits the number and places where errors are logged.
  • Adds testing for error logging.
  • Breaks out the error logging into help methods.

@svevang svevang marked this pull request as ready for review October 15, 2024 15:07
@svevang svevang mentioned this pull request Oct 18, 2024
Open
@svevang svevang requested review from cavis, radical-ube and kookster and removed request for radical-ube and kookster October 18, 2024 16:41
Base automatically changed from asset-processing-count to main October 22, 2024 13:54
cavis
cavis approved these changes Oct 31, 2024
View reviewed changes
Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

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

Couple questions, making sure I understand what the nested rescues are doing. But looks good to me 👍

app/jobs/publish_feed_job.rb Outdated Show resolved Hide resolved
db/migrate/20241002215949_add_asset_processing_count_to_episode_delivery_status.rb Outdated Show resolved Hide resolved
test/models/episode_test.rb Outdated Show resolved Hide resolved
cavis
cavis approved these changes Oct 31, 2024
View reviewed changes
Copy link
Member

@cavis cavis left a comment

Choose a reason for hiding this comment

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

Okay, making more sense after that rebase!

Looks good to me 👍 and good catch on the sync-blocks-rss.

@svevang svevang merged commit d14fa96 into main Nov 4, 2024
3 checks passed
@svevang svevang deleted the limit-error-interface branch November 4, 2024 16:00
svevang added a commit that referenced this pull request Nov 5, 2024
@svevang
Restore the extracted apple delivery coverage
f3eb4f9 
Stray file CI related to #1122
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cavis cavis cavis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@svevang @cavis