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.

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

Implement post-commit actions #323

Merged
merged 3 commits into from
Nov 9, 2023
Merged

Implement post-commit actions #323

merged 3 commits into from
Nov 9, 2023

Conversation

neekolas
Copy link
Contributor

@neekolas neekolas commented Nov 9, 2023

Summary

  • Adds handler for running post-commit actions. Currently the only action is sending welcome messages
  • This will be run as part of the sync process, after messages have been read off the network

@neekolas neekolas marked this pull request as ready for review November 9, 2023 01:03
@neekolas neekolas requested a review from a team November 9, 2023 01:03
}
}
}
let deleter: &mut dyn Delete<StoredGroupIntent, Key = i32> = conn;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to do some gymnastics to make the Delete trait work now that we have multiple structs implementing it. Now that it requires disambiguation it's...not beautiful. Maybe there's some better way to tell the compiler which model we're deleting, or maybe we should rethink the trait to be explicitly generic.

Copy link
Contributor

Choose a reason for hiding this comment

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

p. sure we can use Identifier trait for this, and remove the associated type, ref #314

@neekolas neekolas merged commit ded57cc into main Nov 9, 2023
4 checks passed
@neekolas neekolas deleted the nmolnar/post-commit branch November 9, 2023 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants