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

Refactor config.json settings tables in configuration-settings.rst files to remove horizontal bars #7549

Merged
merged 12 commits into from
Nov 12, 2024

Conversation

guruprasath-v
Copy link
Contributor

Summary

This PR refactors the configuration tables in various .rst files to align with the Mattermost documentation style guide by eliminating horizontal bars in tables.

Changes Made

  • Reformatted Configuration Tables: Updated all config.json entries to the format "Setting" > "Subsetting" across key .rst files, which prevents unwanted horizontal bars and improves readability.
  • Moved Note Content: Extracted note content from tables and added it using .. note:: tags to ensure clarity and consistency.

Files Updated

  • Complete Refactoring:
    • source/configure/authentication-configuration-setttings.rst
    • source/configure/site-configuration-settings.rst
    • source/configure/plugins.configuration-settings.rst
  • Partial Update:
    • source/configure/environment-configuration-settings.rst: Updated one table entry, as most of the documented tables were not present in the codebase.

Additional Information

These changes address all configuration settings found with table format issues, while other configuration files were not modified as they did not contain tables needing refactoring.

@mattermost-build
Copy link
Contributor

Hello @guruprasath-v,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@cwarnermm cwarnermm self-requested a review November 5, 2024 14:40
@cwarnermm cwarnermm added 2: Editor Review Requires review by an editor preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories labels Nov 5, 2024
@cwarnermm
Copy link
Member

@guruprasath-v - Please resolve your merge conflicts in this PR.

Copy link
Contributor Author

@guruprasath-v guruprasath-v left a comment

Choose a reason for hiding this comment

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

Conflict merged

Copy link
Contributor Author

@guruprasath-v guruprasath-v left a comment

Choose a reason for hiding this comment

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

Modified Enabled or Disabled customer portal requests in source/configure/environment-configuration-settings.rst.

@cwarnermm cwarnermm added preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories and removed preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories labels Nov 7, 2024
Copy link

github-actions bot commented Nov 7, 2024

Newest code from guruprasath-v has been published to preview environment for Git SHA e89f2a9

Copy link
Member

@cwarnermm cwarnermm left a comment

Choose a reason for hiding this comment

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

@guruprasath-v - This is an outstanding docs PR that was a BIG lift to complete. Well done and thank you! I've provided editorial and reviewer feedback inline.

In most cases, I've simply removed the bullet point formatting in cases where it isn't needed or preferred.

Please perform a second review looking for any places where there's a single text line note that's currently formatted as a bullet and remove the bullet for consistency.

In a few cases, I've flagged some notes that should be converted to formatted notes/warnings.

source/configure/authentication-configuration-settings.rst Outdated Show resolved Hide resolved
| a single sign-on (SSO) service to create accounts. | - Environment variable: ``MM_EMAILSETTINGS_ENABLESIGNUPWITHEMAIL`` |
+-----------------------------------------------------------------------------------------------+--------------------------------------------------------------------------+
.. note::
- Cloud admins can't modify this configuration setting.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Cloud admins can't modify this configuration setting.
Cloud admins can't modify this configuration setting.

+-----------------------------------------------------------------------------+-------------------------------------------------------------------------+

.. note::
- To provide users with only a single email sign in option on the login page, ensure that the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- To provide users with only a single email sign in option on the login page, ensure that the
To provide users with only a single email sign in option on the login page, ensure that the

source/configure/authentication-configuration-settings.rst Outdated Show resolved Hide resolved
| - **false**: Hides the **Forgot Password** link from the Mattermost login page. | - Environment variable: ``MM_LDAPSETTINGS_FORGOTPASSWORDLINK`` |
+---------------------------------------------------------------------------------+----------------------------------------------------------------------------------+
.. note::
- You can customize the **Forgot Password** link URL by going to **Site Configuration > Customization > Forgot Password Custom Link**.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- You can customize the **Forgot Password** link URL by going to **Site Configuration > Customization > Forgot Password Custom Link**.
You can customize the **Forgot Password** link URL by going to **Site Configuration > Customization > Forgot Password Custom Link**.

| - **false**: **(Default)** Disables previews of SVG files. | - ``config.json`` setting: ``ServiceSettings`` > ``EnableSVGs`` > ``false``|
| | - Environment variable: ``MM_SERVICESETTINGS_ENABLESVGS`` |
+-------------------------------------------------------------------------------+----------------------------------------------------------------------------+
| **Warning**: Enabling SVGs is not recommended in environments where not all users are trusted. |
Copy link
Member

Choose a reason for hiding this comment

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

Please turn this into a formatted warning using .. warning::

| - **false**: **(Default)** Disables rendering in blocks. Instead, LaTeX code is highlighted. | - ``config.json`` setting: ``ServiceSettings`` > ``EnableLatex`` > ``false``|
| | - Environment variable: ``MM_SERVICESETTINGS_ENABLELATEX`` |
+----------------------------------------------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------+
| **Warning**: Enabling LaTeX rendering is not recommended in environments where not all users are trusted. |
Copy link
Member

Choose a reason for hiding this comment

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

Please turn this into a formatted warning

| - **false**: **(Default)** Disables inline rendering of LaTeX. Instead, LaTeX in message text is highlighted. | - ``config.json`` setting: ``ServiceSettings`` > ``EnableInlineLatex`` > ``false`` |
| LaTeX can also be rendered in a code block, if that feature is enabled. See **Enable LaTeX code block rendering**. | - Environment variable: ``MM_SERVICESETTINGS_ENABLEINLINELATEX`` |
+-------------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------+
| **Warning**: Enabling LaTeX rendering isn't recommended in environments where not all users are trusted. |
Copy link
Member

Choose a reason for hiding this comment

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

Please turn this into a formatted warning

Comment on lines 1489 to 1490
- The key must have the YouTube Data API added as a service.
- This key is used in client-side Javascript.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The key must have the YouTube Data API added as a service.
- This key is used in client-side Javascript.
- This key is used in client-side Javascript, and must have the YouTube Data API added as a service.

source/configure/site-configuration-settings.rst Outdated Show resolved Hide resolved
@cwarnermm cwarnermm added the Awaiting Submitter Action Blocked on the author label Nov 7, 2024
@guruprasath-v
Copy link
Contributor Author

guruprasath-v commented Nov 7, 2024

Hello @cwarnermm, I've made the changes as per your feedback, but I had a couple of questions to clarify:

  1. Single Bullet Point in Note:

    • In source/configure/site-configuration-settings.rst at approximately line 1470, under the sub-heading "Maximum Markdown nodes" there’s a .. note:: with just one bullet point :
    .. note::
      - This limit applies to all Mattermost clients, including web, desktop app, and mobile app.
    

    Since it wasn’t explicitly flagged, should I remove the bullet here as well, or leave it as is?

  2. Hypen Usage in Specific Comments:

    • In the same file, at line 1489 (as mentioned in the 66th comment), I see the following:
    .. note::
       - This key is used in client-side Javascript, and must have the YouTube Data API added as a service.
    
    • Similarly, in line 427 (mentioned in the 55th comment), there’s a note:
     .. note::
       - Changing this configuration setting changes the default client language for users who haven't set a language preference via **Settings**. Mattermost applies the user's language preference when specified.
    

    Both of these notes contain a single bullet point. Just confirming: should I keep these bullets as they are, or were they meant to be removed?

Thanks for clarifying!

@cwarnermm
Copy link
Member

Hi @guruprasath-v - Yes, please remove the bullet formatting that you flagged above. Thank you for finding those additional cases.

Please also apply the updates flagged inline above, if you haven't already, starting from #7549 (comment)

guruprasath-v and others added 3 commits November 13, 2024 00:11
- Removed single-item bullets and adjusted formatting as per feedback
- Converted flagged notes into formatted notes/warnings where specified
- Completed all inline updates as per review comments
- Removed single-item bullets and adjusted formatting as per feedback
- Converted flagged notes into formatted notes/warnings where specified
- Completed all inline updates as per review comments
@guruprasath-v
Copy link
Contributor Author

Ma'am, @cwarnermm, I have made all the changes and pushed them to the previous PR. Please check and let me know if any further changes are needed.

@cwarnermm cwarnermm removed the Awaiting Submitter Action Blocked on the author label Nov 12, 2024
@cwarnermm cwarnermm self-requested a review November 12, 2024 21:02
@cwarnermm
Copy link
Member

/update-branch

@mattermost-build
Copy link
Contributor

Error trying to update the PR.
Please do it manually.

@cwarnermm cwarnermm added preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories and removed preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories labels Nov 12, 2024
Copy link

Newest code from guruprasath-v has been published to preview environment for Git SHA 9ddcb90

Copy link
Member

@cwarnermm cwarnermm left a comment

Choose a reason for hiding this comment

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

Thank you, @guruprasath-v!! Really appreciate your help with this docs issue.

@cwarnermm cwarnermm added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Editor Review Requires review by an editor preview-environment Allow the preview environment to be generated for Pull Requests coming from fork repositories labels Nov 12, 2024
@cwarnermm cwarnermm merged commit a97f214 into mattermost:master Nov 12, 2024
5 checks passed
@guruprasath-v guruprasath-v deleted the removing-horizontal-bars branch November 13, 2024 05:05
@guruprasath-v
Copy link
Contributor Author

Hi Ma'am @cwarnermm! Super excited to see our PR merged! Out of curiosity, would this contribution qualify for the Mattermost limited edition mug? 😊

@cwarnermm
Copy link
Member

This is your first Mattermost contribution, isn't it, @guruprasath-v? Yes! First contributions to any Mattermost project area, including documentation, earns you a Mattermost limited edition mug. Thank you again for your hard work on this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request Contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants