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

Add a newline after module options to improve config readability #3402

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

chrzaszcz
Copy link
Member

@chrzaszcz chrzaszcz commented Nov 16, 2021

The resulting config for prod, dev nodes and all test presets should have a single newline between modules in mongooseim.toml. Previously some newlines were missing if there was at least one option as each option is in its separate line and there was no newline after the last option. Maybe for tests it's not that important but for prod it affects the initial configuration file, which had a bit broken layout, leading to worse initial impression.

I chose to use '\n' as literal newlines could make the config spec less readable.

I checked the result manually for mim1, prod and finally for mim1 with the pgsql_mnesia preset enabled.

This way the resulting config for prod, dev nodes and all test presets
has a single newline between each module.
Previously some newlines were missing if there was at least one option
as each option is in its separate line.
@mongoose-im
Copy link
Collaborator

mongoose-im commented Nov 16, 2021

small_tests_24 / small_tests / 82b6c19
Reports root / small


internal_mnesia_24 / internal_mnesia / 82b6c19
Reports root/ big
OK: 1587 / Failed: 0 / User-skipped: 297 / Auto-skipped: 0


small_tests_23 / small_tests / 82b6c19
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 82b6c19
Reports root/ big
OK: 2722 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 82b6c19
Reports root/ big
OK: 2712 / Failed: 1 / User-skipped: 203 / Auto-skipped: 0

sm_SUITE:parallel_manual_ack_freq_1:resume_session_state_stop_c2s
{error,{thrown,{timeout,msg}}}

Report log


ldap_mnesia_24 / ldap_mnesia / 82b6c19
Reports root/ big
OK: 1514 / Failed: 0 / User-skipped: 370 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 82b6c19
Reports root/ big
OK: 2722 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 82b6c19
Reports root/ big
OK: 2722 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 82b6c19
Reports root/ big
OK: 1892 / Failed: 0 / User-skipped: 297 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 82b6c19
Reports root/ big
OK: 1514 / Failed: 0 / User-skipped: 370 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 82b6c19
Reports root/ big
OK: 3121 / Failed: 0 / User-skipped: 183 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 82b6c19
Reports root/ big
OK: 3104 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 82b6c19
Reports root/ big
OK: 3121 / Failed: 0 / User-skipped: 183 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 82b6c19
Reports root/ big
OK: 3121 / Failed: 0 / User-skipped: 183 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 82b6c19
Reports root/ big
OK: 1756 / Failed: 1 / User-skipped: 298 / Auto-skipped: 0

jingle_SUITE:all:resp_4xx_from_sip_proxy_results_in_session_terminate
{error,
  {{assertion_failed,assert,is_iq_result,
     {xmlel,<<"iq">>,
       [{<<"from">>,<<"error.480@localhost">>},
        {<<"to">>,
         <<"alice_resp_4xx_from_sip_proxy_results_in_session_terminate_9.981111@localhost/res1">>},
        {<<"id">>,<<"c1f90c86-7aec-43fd-b2d8-7c378d79ce6d">>},
        {<<"type">>,<<"set">>}],
       [{xmlel,<<"jingle">>,
          [{<<"xmlns">>,<<"urn:xmpp:jingle:1">>},
           {<<"action">>,<<"session-terminate">>},
           {<<"sid">>,<<"b1471975-0dd6-4707-b34a-83feca25ead5">>}],
          [{xmlel,<<"reason">>,[],
             [{xmlel,<<"general-error">>,[],[]},
            {xmlel,<<"sip-error">>,
              [{<<"code">>,<<"480">>}],
              [{xmlcdata,<<"Temporarily Unavailable">>}]}]}]}]},
     "<iq from='error.480@localhost' to='alice_resp_4xx_from_sip_proxy_results_in_session_terminate_9.981111@localhost/res1' id='c1f90c86-7aec-43fd-b2d8-7c378d79ce6d' type='set'><jingle xmlns='urn:xmpp:jingle:1' action='session-terminate' sid='b1471975-0dd6-4707-b34a-83feca25ead5'><reason><general-error/><sip-error code='480'>Temporarily Unavailable</sip-error></reason></jingle></iq>"},
   [{escalus_new_assert,assert_true,2,
      [{file,
         "/home/circleci/app/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
       {line,84}]},
    {jingle_SUITE,send_initiate_and_wait_for_first_iq_set,2,
      [{file,"/home/circleci/app/big_tests/tests/jingle_SUITE.erl"},
       {line,395}]},
    {jingle_SUITE,
      '-resp_...

Report log

@codecov
Copy link

codecov bot commented Nov 16, 2021

Codecov Report

Merging #3402 (82b6c19) into master (eacb079) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3402      +/-   ##
==========================================
+ Coverage   80.90%   80.94%   +0.03%     
==========================================
  Files         414      414              
  Lines       32437    32437              
==========================================
+ Hits        26244    26255      +11     
+ Misses       6193     6182      -11     
Impacted Files Coverage Δ
src/mod_roster_riak.erl 81.53% <0.00%> (-15.39%) ⬇️
src/mod_muc.erl 73.76% <0.00%> (-0.23%) ⬇️
src/pubsub/mod_pubsub.erl 73.19% <0.00%> (-0.12%) ⬇️
src/ejabberd_c2s.erl 89.57% <0.00%> (+0.07%) ⬆️
src/ejabberd_sm.erl 84.91% <0.00%> (+0.32%) ⬆️
src/pubsub/mod_pubsub_db_mnesia.erl 92.85% <0.00%> (+0.42%) ⬆️
src/mod_bosh_socket.erl 78.57% <0.00%> (+1.24%) ⬆️
src/mam/mod_mam_elasticsearch_arch.erl 86.84% <0.00%> (+1.75%) ⬆️
src/mod_bosh.erl 94.36% <0.00%> (+2.11%) ⬆️
src/elasticsearch/mongoose_elasticsearch.erl 84.61% <0.00%> (+7.69%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eacb079...82b6c19. Read the comment docs.

Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

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

Verified locally, it indeed does a better job with newlines 👍🏽

@NelsonVides NelsonVides merged commit 9c57dde into master Nov 17, 2021
@NelsonVides NelsonVides deleted the fix-module-config-newlines branch November 17, 2021 10:12
@Premwoik Premwoik modified the milestones: 5.1.0, 5.0.0 May 25, 2022
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.

None yet

5 participants