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

refactor: wrap error type #203

Merged
merged 7 commits into from
Apr 29, 2024
Merged

refactor: wrap error type #203

merged 7 commits into from
Apr 29, 2024

Conversation

renlulu
Copy link
Contributor

@renlulu renlulu commented Apr 16, 2024

Fixes #116 .

Motivation

Solution

Open questions

@samlaf
Copy link
Collaborator

samlaf commented Apr 22, 2024

@renlulu sorry took us a while to review this, looks good. Just merged another PR which created merge conflict.. really sorry.
Would you mind rebasing on top of master? Then I'll review and we can merge promptly.

@renlulu renlulu force-pushed the refactor/wrap-error-type branch from af0c9b6 to b4c0e60 Compare April 29, 2024 17:18
@renlulu
Copy link
Contributor Author

renlulu commented Apr 29, 2024

@renlulu sorry took us a while to review this, looks good. Just merged another PR which created merge conflict.. really sorry. Would you mind rebasing on top of master? Then I'll review and we can merge promptly.

Yes resolving!

@renlulu renlulu force-pushed the refactor/wrap-error-type branch from b4c0e60 to f5b4427 Compare April 29, 2024 17:36
@renlulu
Copy link
Contributor Author

renlulu commented Apr 29, 2024

@samlaf Hey can you confirm the checking error? Looks like there is some permission issue?

@samlaf
Copy link
Collaborator

samlaf commented Apr 29, 2024 •
edited
Loading

@samlaf Hey can you confirm the checking error? Looks like there is some permission issue?

That's unfortunately expected... the way we upload the badges on the README requires committer to be in eigenlabs org right now.. we have an issue to eventually fix this, but we can merge with this failing.

Will review in a bit.

@renlulu renlulu mentioned this pull request Apr 29, 2024
Merged
3 tasks
samlaf
samlaf requested changes Apr 29, 2024
View reviewed changes
utils/utils.go Outdated Show resolved Hide resolved
utils/utils.go Outdated Show resolved Hide resolved
utils/utils.go Outdated Show resolved Hide resolved
samlaf
samlaf requested changes Apr 29, 2024
View reviewed changes
utils/errors.go Outdated Show resolved Hide resolved
samlaf
samlaf approved these changes Apr 29, 2024
View reviewed changes
Copy link
Collaborator

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

LGTM. reran the flaky test and it passed. Will solve in a separate PR. Let's merge this. :)

@samlaf samlaf merged commit 46fd7b1 into Layr-Labs:master Apr 29, 2024
2 of 3 checks passed
@renlulu renlulu changed the title Refactor/wrap error type refactor: wrap error type Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samlaf samlaf samlaf 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.

Move WrapError to utils
2 participants
@renlulu @samlaf