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

change readthedocs theme to material #2960

Merged
merged 25 commits into from
Dec 2, 2020
Merged

change readthedocs theme to material #2960

merged 25 commits into from
Dec 2, 2020

Conversation

leszke
Copy link
Contributor

@leszke leszke commented Nov 30, 2020

PR is ready and thirsting for your comments.

Includes #2946.
What have I pushed so far, is ready for review. The purpose is to facilitate reviewing process.

I'm also sharing my remarks I got down yesterday. Since it's a drama to comment on GH something that wasn't modified by PR, I'm leaving them this way:

@mongoose-im
Copy link
Collaborator

mongoose-im commented Nov 30, 2020

8833.1 / Erlang 23.0.3 / small_tests / e2dc9f4
Reports root / small


8833.2 / Erlang 23.0.3 / internal_mnesia / e2dc9f4
Reports root/ big
OK: 1513 / Failed: 1 / User-skipped: 168 / Auto-skipped: 0

jingle_SUITE:all:resp_4xx_from_sip_proxy_results_in_session_terminate
{'EXIT',
  {fw_error,
    {ct_framework,error_notification,
      {{case_clause,[]},
       [{ct_framework,error_notification,4,
          [{file,"ct_framework.erl"},{line,997}]},
        {test_server_sup,framework_call,4,
          [{file,"test_server_sup.erl"},{line,778}]},
        {test_server,run_test_case_eval1,6,
          [{file,"test_server.erl"},{line,1268}]},
        {test_server,run_test_case_eval,9,
          [{file,"test_server.erl"},{line,1195}]}]}}}}

Report log


8833.3 / Erlang 23.0.3 / odbc_mssql_mnesia / e2dc9f4
Reports root/ big
OK: 2771 / Failed: 0 / User-skipped: 228 / Auto-skipped: 0


8833.4 / Erlang 23.0.3 / mysql_redis / e2dc9f4
Reports root/ big
OK: 2766 / Failed: 0 / User-skipped: 233 / Auto-skipped: 0


8833.7 / Erlang 23.0.3 / elasticsearch_and_cassandra_mnesia / e2dc9f4
Reports root/ big
OK: 330 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


8833.5 / Erlang 23.0.3 / riak_mnesia / e2dc9f4
Reports root/ big
OK: 1629 / Failed: 0 / User-skipped: 180 / Auto-skipped: 0


8833.6 / Erlang 23.0.3 / ldap_mnesia / e2dc9f4
Reports root/ big
OK: 1405 / Failed: 0 / User-skipped: 258 / Auto-skipped: 0


8833.9 / Erlang 22.3 / pgsql_mnesia / e2dc9f4
Reports root/ big / small
OK: 2784 / Failed: 0 / User-skipped: 215 / Auto-skipped: 0

@codecov
Copy link

codecov bot commented Nov 30, 2020

Codecov Report

Merging #2960 (c06428b) into master (bd3779b) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2960      +/-   ##
==========================================
+ Coverage   79.16%   79.23%   +0.07%     
==========================================
  Files         377      377              
  Lines       32795    32795              
==========================================
+ Hits        25961    25986      +25     
+ Misses       6834     6809      -25     
Impacted Files Coverage Δ
src/ejabberd_local.erl 71.64% <0.00%> (-1.50%) ⬇️
src/global_distrib/mod_global_distrib_utils.erl 65.09% <0.00%> (-0.95%) ⬇️
src/mod_muc_log.erl 77.29% <0.00%> (ø)
src/pubsub/mod_pubsub.erl 71.93% <0.00%> (+0.05%) ⬆️
src/ejabberd_c2s.erl 89.34% <0.00%> (+0.22%) ⬆️
src/mod_muc_room.erl 77.54% <0.00%> (+0.46%) ⬆️
src/ejabberd_sm.erl 76.59% <0.00%> (+0.60%) ⬆️
src/mam/mod_mam_elasticsearch_arch.erl 88.39% <0.00%> (+1.78%) ⬆️
...bal_distrib/mod_global_distrib_hosts_refresher.erl 73.58% <0.00%> (+1.88%) ⬆️
src/ejabberd_zlib.erl 70.27% <0.00%> (+2.70%) ⬆️
... 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 bd3779b...89c3a7c. Read the comment docs.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 1, 2020

8836.1 / Erlang 23.0.3 / small_tests / 3657f53
Reports root / small


8836.4 / Erlang 23.0.3 / mysql_redis / 3657f53
Reports root/ big
OK: 2768 / Failed: 1 / User-skipped: 233 / Auto-skipped: 0

pubsub_SUITE:dag+collection_config:disable_notifications_leaf_test
{'EXIT',
  {fw_error,
    {ct_framework,error_notification,
      {{case_clause,[]},
       [{ct_framework,error_notification,4,
          [{file,"ct_framework.erl"},{line,997}]},
        {test_server_sup,framework_call,4,
          [{file,"test_server_sup.erl"},{line,778}]},
        {test_server,run_test_case_eval1,6,
          [{file,"test_server.erl"},{line,1268}]},
        {test_server,run_test_case_eval,9,
          [{file,"test_server.erl"},{line,1195}]}]}}}}

Report log


8836.3 / Erlang 23.0.3 / odbc_mssql_mnesia / 3657f53
Reports root/ big
OK: 2771 / Failed: 0 / User-skipped: 228 / Auto-skipped: 0


8836.2 / Erlang 23.0.3 / internal_mnesia / 3657f53
Reports root/ big
OK: 1495 / Failed: 0 / User-skipped: 168 / Auto-skipped: 0


8836.7 / Erlang 23.0.3 / elasticsearch_and_cassandra_mnesia / 3657f53
Reports root/ big
OK: 330 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


8836.5 / Erlang 23.0.3 / riak_mnesia / 3657f53
Reports root/ big
OK: 1629 / Failed: 0 / User-skipped: 180 / Auto-skipped: 0


8836.6 / Erlang 23.0.3 / ldap_mnesia / 3657f53
Reports root/ big
OK: 1405 / Failed: 0 / User-skipped: 258 / Auto-skipped: 0


8836.9 / Erlang 22.3 / pgsql_mnesia / 3657f53
Reports root/ big / small
OK: 2784 / Failed: 0 / User-skipped: 215 / Auto-skipped: 0

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.

Amazing work man, huge patience to go through every single file and fix every single indentation, code block, links... I'm enormously thankful 👏

Just a few comments to apply, and then let me know when you think this is finished so we can merge 😄

doc/rest-api/Metrics-backend.md Show resolved Hide resolved
doc/rest-api/Metrics-backend.md Show resolved Hide resolved
doc/user-guide/Bootstrap-Scripts.md Outdated Show resolved Hide resolved
doc/user-guide/How-to-build.md Outdated Show resolved Hide resolved
@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 1, 2020

8838.1 / Erlang 23.0.3 / small_tests / 18f197e
Reports root / small


8838.2 / Erlang 23.0.3 / internal_mnesia / 18f197e
Reports root/ big
OK: 1495 / Failed: 0 / User-skipped: 168 / Auto-skipped: 0


8838.3 / Erlang 23.0.3 / odbc_mssql_mnesia / 18f197e
Reports root/ big
OK: 2771 / Failed: 0 / User-skipped: 228 / Auto-skipped: 0


8838.4 / Erlang 23.0.3 / mysql_redis / 18f197e
Reports root/ big
OK: 2766 / Failed: 0 / User-skipped: 233 / Auto-skipped: 0


8838.5 / Erlang 23.0.3 / riak_mnesia / 18f197e
Reports root/ big
OK: 1629 / Failed: 0 / User-skipped: 180 / Auto-skipped: 0


8838.7 / Erlang 23.0.3 / elasticsearch_and_cassandra_mnesia / 18f197e
Reports root/ big
OK: 330 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


8838.6 / Erlang 23.0.3 / ldap_mnesia / 18f197e
Reports root/ big
OK: 1405 / Failed: 0 / User-skipped: 258 / Auto-skipped: 0


8838.9 / Erlang 22.3 / pgsql_mnesia / 18f197e
Reports root/ big / small
OK: 2784 / Failed: 0 / User-skipped: 215 / Auto-skipped: 0

doc/custom.css Outdated
@@ -2,3 +2,8 @@ img#mim-readme-logo {
margin-right: 1.5em;
margin-bottom: 1em;
}

:root {
--md-primary-fg-color: #459bb6;
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 2, 2020

8848.1 / Erlang 23.0.3 / small_tests / 680c262
Reports root / small


8848.2 / Erlang 23.0.3 / internal_mnesia / 680c262
Reports root/ big
OK: 1495 / Failed: 0 / User-skipped: 168 / Auto-skipped: 0


8848.3 / Erlang 23.0.3 / odbc_mssql_mnesia / 680c262
Reports root/ big
OK: 2771 / Failed: 0 / User-skipped: 228 / Auto-skipped: 0


8848.4 / Erlang 23.0.3 / mysql_redis / 680c262
Reports root/ big
OK: 2766 / Failed: 0 / User-skipped: 233 / Auto-skipped: 0


8848.5 / Erlang 23.0.3 / riak_mnesia / 680c262
Reports root/ big
OK: 1629 / Failed: 0 / User-skipped: 180 / Auto-skipped: 0


8848.6 / Erlang 23.0.3 / ldap_mnesia / 680c262
Reports root/ big
OK: 1405 / Failed: 0 / User-skipped: 258 / Auto-skipped: 0


8848.7 / Erlang 23.0.3 / elasticsearch_and_cassandra_mnesia / 680c262
Reports root/ big
OK: 330 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


8848.9 / Erlang 22.3 / pgsql_mnesia / 680c262
Reports root/ big / small
OK: 2784 / Failed: 0 / User-skipped: 215 / Auto-skipped: 0

Copy link
Contributor

@janciesla8818 janciesla8818 left a comment

Choose a reason for hiding this comment

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

@leszke , @kmakiela great work! I really like the new look and feel of the docs 🎉

@leszke thanks for going so carefully though the documentation and fixing all the links, headers and indentation issues. I really appreciate the effort 💪 Great attention to detail!
I reviewed following sections and they look good as well:

  • developer's guide
  • modules
  • and the commits that follow

I there's only one fix comment for the mod_pubsub, once this is fixed we can approve and merge.


* 3.4.1 alert: **not supported**
* 3.4.2 drop: **not supported**
* 3.4.3 error: **supported**
* 3.4.4 notify: **supported**. Notifications for the `stored` and `direct` conditions are sent as soon as the message has been stored or sent to the recipient.

* 6. Error Handling
6\. Error Handling
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

- 'mod_event_pusher':
- 'mod_event_pusher': 'modules/mod_event_pusher.md'
- 'HTTP backend': 'modules/mod_event_pusher_http.md'
- 'Push backend': 'modules/mod_event_pusher_push.md'
- 'SNS backend': 'modules/mod_event_pusher_sns.md'
- 'RabbitMQ backend': 'modules/mod_event_pusher_rabbit.md'
- 'mod_extdisco': 'modules/mod_extdisco.md'
- 'mod_global_distrib': 'modules/mod_global_distrib.md'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


Implementation of [XEP-0060 (Personal Eventing Protocol)](http://xmpp.org/extensions/xep-0163.html).
Implementation of [XEP-0163 (Personal Eventing Protocol)](http://xmpp.org/extensions/xep-0163.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

great catch!

doc/modules/mod_pubsub.md Show resolved Hide resolved
@leszke
Copy link
Contributor Author

leszke commented Dec 2, 2020

Thank for the review. But before we merge I personally would like somebody to take care and reply to my concerns I shared in PR description 🙂

@janciesla8818
Copy link
Contributor

Comments on the concerns from the PR description below:

* http://localhost:8000/#public-testing -> we’ve lost load test history image from tide

Please create a ticket for that so that we don't forget about it when we decide how to proceed with tide.

* http://localhost:8000/user-guide/Getting-started/#note-about-session-conflicts -> this note is formatted as a subsection, therefore incorporated into the table of contents, should it be?

Unless there would be some special formatting for the notes, let's leave it as subsection. It goes into the table of contents as it is a header and we want to have a header there.

* http://localhost:8000/user-guide/push-notifications/Push-notifications/ -> in XEP-0357 chapter, do we really need these redundant links in every paragraph? same applies to another files tbh

I could see them being useful for the user. No harm in leaving them. I actually prefer to have a bit more links than not enough.

* http://localhost:8000/Contributions/ -> almost all the PRs are still open / closed not merged - is it actual contribution so?

Still contribution. I went though the list and plenty of PR were merged, some were cherry picked and some are still hanging. Even the open and closed ones are still valid contributions that it's good to have there even for our reference.

* http://localhost:8000/advanced-configuration/auth/ -> the beginning and the last chapter seem redundant

I don't have the same feeling about this section. This is the top page that introduces all the authentication related parts. It's good that it has intro and link method specific options. It may seem redundant but I find it useful from user perspective.

* http://localhost:8000/advanced-configuration/Modules/#mod_muc_light -> the linked PR is closed not merged - should it be?

I could not find the PR you mention, could you point to the exact line?

* http://localhost:8000/developers-guide/Hooks-and-handlers/#creating-your-own-hooks -> Gentlemen, we have a todo left by @arcusfelis

Could you create a ticket for that, so we don't forget about it

@chrzaszcz
Copy link
Member

http://localhost:8000/#public-testing -> we’ve lost load test history image from tide
http://localhost:8000/Contributions/ -> almost all the PRs are still open / closed not merged - is it actual contribution so?
http://localhost:8000/advanced-configuration/Modules/#mod_muc_light -> the linked PR is closed not merged - should it be?
http://localhost:8000/developers-guide/Hooks-and-handlers/#creating-your-own-hooks -> Gentlemen, we have a todo left by @arcusfelis

I would log these in one issue not to multiply them too much.

http://localhost:8000/user-guide/Getting-started/#note-about-session-conflicts -> this note is formatted as a subsection, therefore incorporated into the table of contents, should it be?

For this one, the nesting level is 4, why it looks odd, I think it should be 3 for consistency.

http://localhost:8000/user-guide/push-notifications/Push-notifications/ -> in XEP-0357 chapter, do we really need these redundant links in every paragraph? same applies to another files tbh
http://localhost:8000/advanced-configuration/auth/ -> the beginning and the last chapter seem redundant

I think these ones don't need any actions, they look fine.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 2, 2020

8849.1 / Erlang 23.0.3 / small_tests / 8cf534a
Reports root / small


8849.2 / Erlang 23.0.3 / internal_mnesia / 8cf534a
Reports root/ big
OK: 1495 / Failed: 0 / User-skipped: 168 / Auto-skipped: 0


8849.4 / Erlang 23.0.3 / mysql_redis / 8cf534a
Reports root/ big
OK: 2766 / Failed: 0 / User-skipped: 233 / Auto-skipped: 0


8849.3 / Erlang 23.0.3 / odbc_mssql_mnesia / 8cf534a
Reports root/ big
OK: 2771 / Failed: 0 / User-skipped: 228 / Auto-skipped: 0


8849.7 / Erlang 23.0.3 / elasticsearch_and_cassandra_mnesia / 8cf534a
Reports root/ big
OK: 330 / Failed: 0 / User-skipped: 38 / Auto-skipped: 0


8849.5 / Erlang 23.0.3 / riak_mnesia / 8cf534a
Reports root/ big
OK: 1629 / Failed: 0 / User-skipped: 180 / Auto-skipped: 0


8849.6 / Erlang 23.0.3 / ldap_mnesia / 8cf534a
Reports root/ big
OK: 1405 / Failed: 0 / User-skipped: 258 / Auto-skipped: 0


8849.9 / Erlang 22.3 / pgsql_mnesia / 8cf534a
Reports root/ big / small
OK: 2784 / Failed: 0 / User-skipped: 215 / Auto-skipped: 0

@arcusfelis
Copy link
Contributor

@leszke that part from TODO https://github.com/esl/MongooseIM/blob/c06428b2d3c6d1b5e3fdd1013f44a0cd1ec216d0/doc/developers-guide/Hooks-and-handlers.md :

(mongooseim@localhost)1> gen_mod:is_loaded(<<"localhost">>, mod_hook_example).
false
(mongooseim@localhost)2> gen_mod:start_module(<<"localhost">>, mod_hook_example, [no_opts]).
{ok,ok}
(mongooseim@localhost)3> gen_mod:is_loaded(<<"localhost">>, mod_hook_example).
true
(mongooseim@localhost)4> mongoose_logs:set_module_loglevel(mod_hook_example, info).
ok
(mongooseim@localhost)5> mod_hook_example:run_custom_hook(<<"localhost">>).
when=2020-12-02T10:29:14.875981+00:00 level=error what=first_handler pid=<0.2321.0> at=mod_hook_example:first_handler/2:40 value=5 result=7 argument=2
when=2020-12-02T10:29:14.876248+00:00 level=error what=stopping_handler pid=<0.2321.0> at=mod_hook_example:stopping_handler/2:46 value=7 result=9 argument=2
ok
when=2020-12-02T10:29:14.876453+00:00 level=error what=hook_finished pid=<0.2321.0> at=mod_hook_example:run_custom_hook/1:35 result_acc_{example,value}=9 result_acc_timestamp=1606904954871246 result_acc_stanza=undefined result_acc_ref=#Ref<0.186499973.2771648514.42380> result_acc_origin_stanza=undefined result_acc_origin_pid=<0.2321.0> result_acc_origin_location_mfa={mod_hook_example,run_custom_hook,1} result_acc_origin_location_line=31 result_acc_origin_location_file=/Users/mikhailuvarov/erlang/esl/MongooseIM/src/mod_hook_example.erl result_acc_non_strippable= result_acc_mongoose_acc=true result_acc_lserver=localhost result=9

@leszke
Copy link
Contributor Author

leszke commented Dec 2, 2020

@janciesla8818

* http://localhost:8000/advanced-configuration/Modules/#mod_muc_light -> the linked PR is closed not merged - should it be?

I could not find the PR you mention, could you point to the exact line?

😁 the art of camouflage is strong with this one. It is in the one and only line of this section, under the link to XEP which is actually the link to the PR itself.

@leszke
Copy link
Contributor Author

leszke commented Dec 2, 2020

I ran regex through the files and unified the way XEPs are shown, also spotted yet another broken link-that-wasn't-actually-the-link 💪 in mod_event_pusher_push. Ultimately I stored the remaining things which we want to take care of in the future in this ticket. Once the CI is done, I consider the PR to be ready to merge 🔥

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.

Absolutely amazing ❤️

@NelsonVides NelsonVides merged commit ed39d02 into master Dec 2, 2020
@NelsonVides NelsonVides deleted the lw/docs branch December 2, 2020 13:36
@leszke leszke added this to the 4.1.0 milestone Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants