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

Added sync on_resolve extension. #7

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Added sync on_resolve extension. #7

merged 1 commit into from
Mar 28, 2024

Conversation

trollfot
Copy link
Contributor

No description provided.

@trollfot trollfot mentioned this pull request Mar 25, 2024
Closed
@ThirVondukr
Copy link
Owner

Looks good, I'll make a couple of changes if you don't mind - I think it should be possible to add both (sync and async) on_resolve methods to an extension class, so on_resolve should be renamed to on_resolve_async.
There are also some minor formatting issues but I can fix them myself (or you can install taskfile to run all the tasks and fix them yourself too.

@ThirVondukr
Copy link
Owner

@trollfot Just pinging you in case you didn't see the message, I can merge the branch and change some things that I want, if you don't mind.

@trollfot
Copy link
Contributor Author

@ThirVondukr Hey. Sorry, I got swamped today. I can try and do the changes tomorrow !

@trollfot
Copy link
Contributor Author

"I can merge the branch and change some things that I want, if you don't mind." If you feel like it's not a problem for you to spend time on it, please go for it, i'm happy either way

@ThirVondukr
Copy link
Owner

No, it's not a problem, gonna finish it tomorrow

@ThirVondukr ThirVondukr changed the base branch from main to sync-on-resolve March 28, 2024 08:19
@ThirVondukr ThirVondukr merged commit 9d1b78e into ThirVondukr:sync-on-resolve Mar 28, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
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
@trollfot @ThirVondukr