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

feat(*): correct panic error #758

Merged
merged 9 commits into from
May 6, 2024
Merged

feat(*): correct panic error #758

merged 9 commits into from
May 6, 2024

Conversation

linhbkhn95
Copy link
Contributor

@linhbkhn95 linhbkhn95 commented Oct 8, 2023 •
edited
Loading

Background

Currently, the Panic error show seems redundant with the processor error log.

Why did we need it?

Correct panic error msg

Issue Link

#745

@linhbkhn95 linhbkhn95 mentioned this pull request Oct 8, 2023
Open
@codecov
Copy link

codecov bot commented Oct 8, 2023 •
edited
Loading

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 68.26%. Comparing base (8df0bfa) to head (d00e218).
Report is 2 commits behind head on master.

❗ Current head d00e218 differs from pull request most recent head 4cee58e. Consider uploading reports for the commit 4cee58e to get more accurate results

Files Patch % Lines
processor.go 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #758      +/-   ##
==========================================
- Coverage   68.41%   68.26%   -0.16%     
==========================================
  Files          27       27              
  Lines        3841     3841              
==========================================
- Hits         2628     2622       -6     
- Misses        929      934       +5     
- Partials      284      285       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kamikazechaser
kamikazechaser approved these changes Oct 19, 2023
View reviewed changes
Copy link
Collaborator

@kamikazechaser kamikazechaser left a comment

Choose a reason for hiding this comment

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

LGTM

cc/ @hibiken

@linhbkhn95 linhbkhn95 mentioned this pull request Dec 12, 2023
Merged
@hibiken
Copy link
Owner

hibiken commented Dec 16, 2023

@linhbkhn95 Would you mind rebasing the branch and resolving conflicts?

@linhbkhn95
Copy link
Contributor Author

@hibiken I rebased and resolved the conflict. Please merge it

@kamikazechaser
Copy link
Collaborator

kamikazechaser commented Jan 26, 2024 •
edited
Loading

@hibiken Good fix, ready for merge. I'm prepping v0.25.0 but we should include this.

@kamikazechaser kamikazechaser mentioned this pull request Jan 29, 2024
Closed
12 tasks
@hibiken
Copy link
Owner

hibiken commented May 1, 2024

LGTM - please resolve conflicts and merge

@linhbkhn95
Copy link
Contributor Author

don't have any conflict, can you check it? @hibiken

@kamikazechaser kamikazechaser merged commit 1740088 into hibiken:master May 6, 2024
5 of 7 checks passed
dusty-cjh pushed a commit to dusty-cjh/asynq that referenced this pull request Jul 10, 2024
@linhbkhn95
feat(*): correct panic error (hibiken#758)
3cc532a 
* error panic handling

* updated CHANGELOG.md file

* correct msg panic error (hibiken#5)

* correct msg panic error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamikazechaser kamikazechaser kamikazechaser approved these changes

Assignees
No one assigned
Labels
pr-bugfix ready-to-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@linhbkhn95 @hibiken @kamikazechaser