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

docs: improve the documentation #210

Merged
merged 8 commits into from
Jul 10, 2024
Merged

docs: improve the documentation #210

merged 8 commits into from
Jul 10, 2024

Conversation

dcoa
Copy link
Contributor

@dcoa dcoa commented Jun 11, 2024

This PR improves the repository documentation for a better understanding of the eox-tenant plugin.

For the plugin description

Plugin for managing multiple tenants (organizations) within a single Open edX instance, replacing the microsites and site configuration features with a more robust multi-tenancy model.

Proposal

➡️ Maintain the settings configuration for the current release.

  • Simplification: Find the necessary information without being distracted by details that may no longer be relevant (e.g. Ironwood).
  • Relevance: Users typically work with recent versions of software. Documenting the latest settings ensures that our documentation is pertinent and useful for most users.
  • Maintenance: We lower the risk of outdated information and reduce the effort required to keep the documentation current. (e.g., before Redwood we have that the current configuration works for Juniper, that is not true because the latest version at that moment doesn't support that release).
  • Accuracy: During the migration process, we are adding the name of the previous release in the documentation even when the settings are the same, which could lead to unnecessary action for the user.

Also, the packages management like (pypi or npm - use the readme and the latest release to explain the documentation) if you want to install another version you should select it and that will show you the relevant information for that tag (readme for that version). That behavior is another motivation for this change, showing the information relevant to the version that the user wants to use.

Additional information
JIRA

@Alec4r Alec4r requested review from Alec4r and Asespinel June 11, 2024 10:26
@dcoa dcoa marked this pull request as ready for review June 12, 2024 01:54
@dcoa dcoa requested a review from a team June 12, 2024 01:55
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@dcoa dcoa changed the title docs: init improvements in readme docs: improve the documentation Jun 13, 2024
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@dcoa dcoa force-pushed the dcoa/improve-docs branch 3 times, most recently from 0a27346 to a29fb7c Compare June 24, 2024 05:34
mariajgrimaldi
mariajgrimaldi previously approved these changes Jun 25, 2024
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@magajh
Copy link
Contributor

magajh commented Jul 4, 2024

Proposal

➡️ Maintain the settings configuration for the current release.

  • Simplification: Find the necessary information without being distracted by details that may no longer be relevant (e.g. Ironwood).
  • Relevance: Users typically work with recent versions of software. Documenting the latest settings ensures that our documentation is pertinent and useful for most users.
  • Maintenance: We lower the risk of outdated information and reduce the effort required to keep the documentation current. (e.g., before Redwood we have that the current configuration works for Juniper, that is not true because the latest version at that moment doesn't support that release).
  • Accuracy: During the migration process, we are adding the name of the previous release in the documentation even when the settings are the same, which could lead to unnecessary action for the user.

Also, the packages management like (pypi or npm - use the readme and the latest release to explain the documentation) if you want to install another version you should select it and that will show you the relevant information for that tag (readme for that version). That behavior is another motivation for this change, showing the information relevant to the version that the user wants to use.

Hi @dcoa,

Thank you for the detailed explanation in the PR description regarding the removal of setting configurations for previous Open edX releases. Your points about simplification, relevance, maintenance, and accuracy are well noted.

However, I have a few thoughts:

  1. If we are including an Open edX named version in the compatibility table, I think we should keep documentation of the settings necessary for successfully using this plugin with that version. That's why I'm not entirely convinced about removing all previous settings.

  2. If we are going to direct users to check the release notes, we need to ensure we consistently include the setting config for the supported Open edX versions in the release notes of a new eox-core version, or at least when there are breaking changes requiring updates in the configuration for a specific named version. This should be documented in our release strategy

I think it's a good idea to include this information in the release notes from now on. Maybe we could keep the currently listed settings configurations in a document on RTD and just provide a link to it in this README so users can check it if needed?

@Alec4r
Copy link
Member

Alec4r commented Jul 4, 2024

Proposal
➡️ Maintain the settings configuration for the current release.

  • Simplification: Find the necessary information without being distracted by details that may no longer be relevant (e.g. Ironwood).
  • Relevance: Users typically work with recent versions of software. Documenting the latest settings ensures that our documentation is pertinent and useful for most users.
  • Maintenance: We lower the risk of outdated information and reduce the effort required to keep the documentation current. (e.g., before Redwood we have that the current configuration works for Juniper, that is not true because the latest version at that moment doesn't support that release).
  • Accuracy: During the migration process, we are adding the name of the previous release in the documentation even when the settings are the same, which could lead to unnecessary action for the user.

Also, the packages management like (pypi or npm - use the readme and the latest release to explain the documentation) if you want to install another version you should select it and that will show you the relevant information for that tag (readme for that version). That behavior is another motivation for this change, showing the information relevant to the version that the user wants to use.

Hi @dcoa,

Thank you for the detailed explanation in the PR description regarding the removal of setting configurations for previous Open edX releases. Your points about simplification, relevance, maintenance, and accuracy are well noted.

However, I have a few thoughts:

1. If we are including an Open edX named version in the compatibility table, I think we should keep documentation of the settings necessary for successfully using this plugin with that version. That's why I'm not entirely convinced about removing all previous settings.

2. If we are going to direct users to check the release notes, we need to ensure we consistently include the setting config for the supported Open edX versions in the release notes of a new eox-core version, or at least when there are breaking changes requiring updates in the configuration for a specific named version. This should be documented in our [release strategy](https://github.com/eduNEXT/edunext-internal-documentation/pull/194)

I think it's a good idea to include this information in the release notes from now on. Maybe we could keep the currently listed settings configurations in a document on RTD and just provide a link to it in this README so users can check it if needed?

Hi,

I have been thinking about just adding the link to the tag/documentation should be enough: e.g: https://github.com/eduNEXT/eox-tenant/blob/v6.2.0/README.rst, at least for releases that are not compatible with the current version.

Because if the table show that nutmeg is compatible with versions greater than or equal to 6.2, it means the latest version 11.7.0 is compatible also.

Its assumed that a version is compatible by default if the user doesn't have to change any setting, if the user has to do something, we should add it to our docs.

However, remember that we have "contract" to support the previous release and current release, if a release is 2 version before the current, we could try but is not mandatory.

@dcoa
Copy link
Contributor Author

dcoa commented Jul 5, 2024

To clarify, I will use “version” to talk about eox-tenant and release to refer to Open edX.

I'm not 100% sure if I caught your concerns but those are my thoughts:

@Alec4r, the contract is still maintained, this repository is a good example of it, the changes for Maple, which is the oldest release supported for 11.x, are listed. Nutmeg, Palm, and Olive are not listed because they use the same configuration as Quince does.

I was thinking of the major release (for the settings list), and the paragraph description does not explain that, sorry for that.

Maybe we could keep the currently listed settings configurations in a document on RTD and just provide a link to it in this README so users can check it if needed?

@magajh, I'm not against this idea, even at some point we have to do that because of the length that section would have in the future.

The compatibility notes table should exist as it is because that gives us an idea of which version to install for each Open edX release, we agree about that. However, from my point of view ,the usage section should describe the information for the eox-tenant version that is displayed. If the user wants to use version 6.x, they can visit that tag and the readme gives the corresponding settings.

Let's use Django documentation as an example, when you open it, the latest release (5.0) is displayed if you are using a previous one, you should select the version, that changes the documentation. (I think is the same but inside GitHub).

Of course, the most important here is to have documentation useful for anyone and you have a better understanding of what is necessary, I'm open to making any relevant changes. 😁

@magajh magajh self-requested a review July 7, 2024 20:00
magajh
magajh previously approved these changes Jul 7, 2024
Copy link
Contributor

@magajh magajh left a comment

Choose a reason for hiding this comment

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

@dcoa @Alec4r thank you both for your comments!

@dcoa I agree that this README is good as it is. I don't think we need to make any more changes, except for one suggestion in the section where we explain what happens if the version the user wants is not listed, for better understanding.

Please ping me if you need my approval again.

README.rst Outdated Show resolved Hide resolved
@dcoa dcoa dismissed stale reviews from magajh and mariajgrimaldi via b8c0963 July 8, 2024 04:02
@dcoa dcoa force-pushed the dcoa/improve-docs branch from a29fb7c to b8c0963 Compare July 8, 2024 04:02
@magajh magajh self-requested a review July 8, 2024 19:00
magajh
magajh previously approved these changes Jul 8, 2024
README.rst Outdated Show resolved Hide resolved
Co-authored-by: Bryann Valderrama <[email protected]>
Copy link
Contributor

@BryanttV BryanttV left a comment

Choose a reason for hiding this comment

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

LGTM!

@magajh magajh self-requested a review July 10, 2024 20:54
@dcoa dcoa merged commit c4b4b6c into master Jul 10, 2024
11 checks passed
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.

6 participants