-
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
C2S features small optimisation #4077
Conversation
This way we can reduce a little bit the duplication of filtering code between c2s and c2s_stanzas
small_tests_24 / small_tests / 4650328 elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 4650328 small_tests_25_arm64 / small_tests / 4650328 small_tests_25 / small_tests / 4650328 ldap_mnesia_24 / ldap_mnesia / 4650328 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4650328 ldap_mnesia_25 / ldap_mnesia / 4650328 dynamic_domains_mysql_redis_25 / mysql_redis / 4650328 pgsql_mnesia_24 / pgsql_mnesia / 4650328 dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 4650328 dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 4650328 internal_mnesia_25 / internal_mnesia / 4650328 pgsql_mnesia_25 / pgsql_mnesia / 4650328 mysql_redis_25 / mysql_redis / 4650328 mssql_mnesia_25 / odbc_mssql_mnesia / 4650328 |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #4077 +/- ##
==========================================
+ Coverage 83.87% 83.91% +0.03%
==========================================
Files 527 527
Lines 33150 33152 +2
==========================================
+ Hits 27806 27819 +13
+ Misses 5344 5333 -11
☔ View full report in Codecov by Sentry. |
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.
👍
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.
looks fine, probably would use Mech variable instead of M.
Two things here:
c2s_stream_features
can already take the initial list of pre-created features as an argument, instead of giving an empty list and then appending two lists. This way, the initial list can also be picked up from subsequent handlers and modified – which SASL2 will do with SASL1 ascribed earlier.cyrsasl:listmech/1
output is given tomongoose_c2s:filter_mechanism/1
twice, once in c2s itself, and another one when creating the features in c2s_stanza. Unify them.