-
Notifications
You must be signed in to change notification settings - Fork 428
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
Refactored hook handlers in ejabberd_s2s module #3762
Conversation
small_tests_24 / small_tests / 723d58b small_tests_25 / small_tests / 723d58b dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 723d58b ldap_mnesia_24 / ldap_mnesia / 723d58b dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 723d58b dynamic_domains_mysql_redis_25 / mysql_redis / 723d58b ldap_mnesia_25 / ldap_mnesia / 723d58b pgsql_mnesia_24 / pgsql_mnesia / 723d58b dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 723d58b muc_SUITE:hibernation:hibernated_room_can_be_queried_for_archive{error,{{assertion_failed,assert,is_groupchat_message,
[<<"Restorable message">>],
undefined,"undefined"},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{muc_SUITE,wait_for_mam_result,3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4394}]},
{muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4130}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{muc_SUITE,hibernated_room_can_be_queried_for_archive,1,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4126}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} internal_mnesia_25 / internal_mnesia / 723d58b elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 723d58b pgsql_mnesia_25 / pgsql_mnesia / 723d58b mysql_redis_25 / mysql_redis / 723d58b mssql_mnesia_25 / odbc_mssql_mnesia / 723d58b push_integration_SUITE:pubsub_ful:pm_notifications_with_inbox:inbox_msg_unread_count_apns{error,
{{assertMatch,
[{module,push_integration_SUITE},
{line,662},
{expression,"Data"},
{pattern,"# { << \"message-count\" >> := ExpectedCount }"},
{value,
#{<<"last-message-body">> => <<"Private message">>,
<<"last-message-sender">> =>
<<"alice_inbox_msg_unread_count_apns_2693@localhost">>,
<<"message-count">> => 1}}]},
[{push_integration_SUITE,check_notification,2,
[{file,
"/home/circleci/project/big_tests/tests/push_integration_SUITE.erl"},
{line,662}]},
{push_integration_SUITE,'-inbox_msg_unread_count/3-fun-0-',6,
[{file,
"/home/circleci/project/big_tests/tests/push_integration_SUITE.erl"},
{line,578}]},
{escalus_story,story,4,
[{file,
"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1291}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1223}]}]}} riak_mnesia_24 / riak_mnesia / 723d58b dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 723d58b mssql_mnesia_25 / odbc_mssql_mnesia / 723d58b |
Codecov ReportBase: 81.27% // Head: 81.27% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #3762 +/- ##
=======================================
Coverage 81.27% 81.27%
=======================================
Files 532 532
Lines 34067 34067
=======================================
Hits 27689 27689
Misses 6378 6378 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
But I'm also now thinking, migrating module by module means the reviewer might forget, or not know, if a certain hook had already been build in the backwards compatible way. And if that hook happens not to be touched by tests for whatever bad reason, then we have a risk of merging something invalid. Not saying this is a big risk, I'm just now hesitating what is the least confusing and most incremental way to proceed with the hooks migration. Keep it going per module, but we should keep in mind also what has been adapted already 🤔
I like such small PRs :) |
This PR changes all hook handlers in
ejabberd_s2s
module togen_hook
format.