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 PDF: add release-8.2 and remove archived versions #3020

Merged
merged 9 commits into from
Jul 3, 2024

Conversation

qiancai
Copy link
Contributor

@qiancai qiancai commented Jul 3, 2024

PR Type

configuration changes


Description

  • Updated branch patterns in docs-cn-postsubmits.yaml and docs-postsubmits.yaml to include support for release-8.2.
  • Removed support for release-5.0 and release-6.6 in both files.

Changes walkthrough 📝

Relevant files
Configuration changes
docs-cn-postsubmits.yaml
Update branch patterns for docs-cn postsubmits                     

prow-jobs/pingcap/docs/docs-cn-postsubmits.yaml

  • Updated branch patterns for postsubmits.
  • Added support for release-8.2.
  • Removed support for release-5.0 and release-6.6.
  • +4/-4     
    docs-postsubmits.yaml
    Update branch patterns for docs postsubmits                           

    prow-jobs/pingcap/docs/docs-postsubmits.yaml

  • Updated branch patterns for postsubmits.
  • Added support for release-8.2.
  • Removed support for release-5.0 and release-6.6.
  • +4/-4     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the pull request title and diff, it appears that the changes are related to updating the release versions in the documentation. The changes are limited to updating the regular expressions used for branch matching in the YAML files.

    There are no potential problems with this pull request since it is just a documentation update and it does not affect the codebase or the CI/CD pipeline.

    As for fixing suggestions, since this is just a documentation update, there are no changes required. The pull request can be merged as is. However, it is a good practice to ensure that the changes made are tested before merging.

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    Hi @qiancai. Thanks for your PR.

    I'm waiting for a PingCAP-QE member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

    Once the patch is verified, the new status will be reflected by the ok-to-test label.

    I understand the commands that are listed here.

    Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 3, 2024

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review None

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 3, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Include release-5.0 in the regex if it is still supported

    The regex for release-5.[1-4] excludes the release-5.0 version, which was included in the
    previous pattern. If release-5.0 is still under support and needs to be included in the
    postsubmit jobs, the regex should be adjusted to reflect this.

    prow-jobs/pingcap/docs/docs-postsubmits.yaml [13]

    -- ^release-5\.[1-4]$
    +- ^release-5\.[0-4]$
     
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: The suggestion correctly points out that the new regex release-5.[1-4] excludes release-5.0, which was previously included. If release-5.0 is still supported, this is a critical oversight, and the regex should be adjusted to release-5.[0-4] to include it.

    10
    Adjust the regex to include all necessary minor versions for releases 6 and 7

    It appears that the regex for the release branches has been modified to exclude certain
    versions that were previously included. Specifically, the regex for release-6.[15] and
    release-7.[156] might unintentionally exclude versions like release-6.2 or release-7.2,
    which were included in the previous pattern. Consider adjusting the regex to include all
    necessary minor versions if those versions still need support.

    prow-jobs/pingcap/docs/docs-cn-postsubmits.yaml [14-15]

    -- ^release-6\.[15]$
    -- ^release-7\.[156]$
    +- ^release-6\.[1-5]$
    +- ^release-7\.[1-6]$
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies that the new regex patterns for release-6.[15] and release-7.[156] exclude certain versions that were previously included. Adjusting the regex to release-6.[1-5] and release-7.[1-6] ensures all necessary minor versions are covered, which is crucial for maintaining proper support.

    9
    Maintainability
    Synchronize regex patterns across different configuration files for consistency

    Consider maintaining consistency in the regex patterns across different configuration
    files to ensure uniform behavior. The patterns for branch releases should ideally be
    synchronized between docs-cn-postsubmits.yaml and docs-postsubmits.yaml unless
    intentionally configured otherwise.

    prow-jobs/pingcap/docs/docs-postsubmits.yaml [14-15]

    -- ^release-6\.[15]$
    -- ^release-7\.[156]$
    +- ^release-6\.[1-5]$
    +- ^release-7\.[1-6]$
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Maintaining consistency in regex patterns across configuration files is important for uniform behavior and maintainability. This suggestion ensures that the patterns for branch releases are synchronized, which helps prevent configuration errors and inconsistencies.

    8
    Enhancement
    Ensure all necessary patches for release 8 are included in the regex

    The addition of release-8.[0-2] is a good update, but ensure that all necessary patches
    for these versions are considered. If patches like release-8.3 are expected, they should
    be included in the regex.

    prow-jobs/pingcap/docs/docs-cn-postsubmits.yaml [16]

    -- ^release-8\.[0-2]$
    +- ^release-8\.[0-3]$
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion to include release-8.3 in the regex is valid if future patches for release 8 are expected. However, without specific knowledge of the release plans, this suggestion is more of a precautionary enhancement rather than a critical fix.

    7

    Co-authored-by: Grace Cai <qqzczy@126.com>
    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the Pull Request description and the provided diff, it seems that this PR is a configuration change that updates branch patterns in docs-cn-postsubmits.yaml and docs-postsubmits.yaml to include support for release-8.2 and removes support for release-5.0 and release-6.6 in both files.

    The key changes in this PR are as follows:

    • Updated branch patterns in docs-cn-postsubmits.yaml and docs-postsubmits.yaml to include support for release-8.2.
    • Removed support for release-5.0 and release-6.6 in both files.

    There seem to be no potential problems with this PR, as it is just a configuration change.

    However, it is good to ensure that the configurations are tested and validated before merging the PR to the main branch.

    Suggestions:

    • Validate the configurations before merging the PR.
    • Add a comment to the PR requesting to validate the configurations.
    • Once the configurations are validated, merge the PR to the main branch.

    Co-authored-by: Grace Cai <qqzczy@126.com>
    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the PR title and description, it seems like the changeset is to update the branch patterns in docs-cn-postsubmits.yaml and docs-postsubmits.yaml to include support for release-8.2 and remove support for release-5.0 and release-6.6.

    The changes look good and there are no potential problems that I can see. It's always good to keep the supported releases up-to-date, and removing the unsupported versions will make the codebase cleaner.

    My suggestion would be to merge the PR after the CI checks pass.

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the PR title and description, the changes in this PR are related to updating the branch patterns in the docs-cn-postsubmits.yaml and docs-postsubmits.yaml files. Specifically, this PR adds support for release-8.2 and removes support for release-5.0 and release-6.6 in both files.

    The changes look good and there shouldn't be any potential problems. However, it's always good to double-check if the removed versions are not being used by anyone or any system.

    As for fixing suggestions, I don't have any since the changes appear to be straightforward and correct. However, it's always a good idea to verify the changes locally before merging the PR.

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the PR title and description, the key changes made include updating the branch patterns in docs-cn-postsubmits.yaml and docs-postsubmits.yaml to include support for release-8.2 and removing support for release-5.0 and release-6.6 in both files.

    The changes look straightforward and should not cause any issues. However, it is always a good idea to double-check the changes before merging.

    One potential problem is that the regex patterns in the branches section may not cover all possible branch names, especially for future releases. It might be a good idea to use a more generic regex pattern that can match all future release branches as well.

    Suggestions for fixing:

    • Double-check the changes made and ensure they are correct.
    • Consider using more generic regex patterns in the branches section to future-proof the configuration. For example, using ^release-\d+\.\d+$ to match all release branches.

    Copy link
    Collaborator

    @wuhuizuo wuhuizuo left a comment

    Choose a reason for hiding this comment

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

    /lgtm
    /ok-to-test
    /approve

    Co-authored-by: Grace Cai <qqzczy@126.com>
    @ti-chi-bot ti-chi-bot bot removed the lgtm label Jul 3, 2024
    Co-authored-by: Grace Cai <qqzczy@126.com>
    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the pull request title, the changeset is focused on updating the documentation PDF to include the release-8.2 and removing support for the archived versions like release-5.0 and release-6.6.

    The changes seem to be well documented and the code changeset looks good. However, there might be a potential problem when removing support for older versions like release-5.0 and release-6.6, as some users might still be using these versions. So it is recommended to communicate to the users about the removal of support for the old versions.

    As for fixing suggestions, it is recommended to add a note to the documentation or the release notes indicating that support for release-5.0 and release-6.6 has been removed and also recommend users to upgrade to the latest release version. Additionally, it is recommended to test the changes before merging to ensure that the documentation PDF is generated with the expected changes.

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the PR title and description, the changes made in this PR are related to updating the branch patterns in docs-cn-postsubmits.yaml and docs-postsubmits.yaml files. The update includes adding support for release-8.2 and removing support for release-5.0 and release-6.6 in both files.

    There are no potential problems identified in this PR.

    However, I suggest adding a short description of why the changes are needed to the PR description. This will help other reviewers to understand the purpose of the changes.

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the pull request title and description, it seems that the changes only affect the documentation configuration files docs-cn-postsubmits.yaml and docs-postsubmits.yaml. The changes include updating the branch patterns to include support for release-8.2, and removing support for release-5.0 and release-6.6 in both files.

    The pull request seems to be straightforward and does not introduce any major problems. However, there are a few suggestions for improvement:

    • It would be helpful to include a brief explanation in the pull request description about why support for release-5.0 and release-6.6 were removed.
    • It may be beneficial to add a comment in the configuration files explaining the changes made and why they were made.
    • It would be good to check whether any other features of the documentation were affected by these changes, and if so, to ensure they still function as expected.

    Overall, the pull request seems to be well-constructed and beneficial. It updates the configuration files to support the latest release of the product and removes support for older, inactive releases.

    @wuhuizuo
    Copy link
    Collaborator

    wuhuizuo commented Jul 3, 2024

    /lgtm

    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    [APPROVALNOTIFIER] This PR is APPROVED

    This pull-request has been approved by: wuhuizuo

    The full list of commands accepted by this bot can be found here.

    The pull request process is described here

    Needs approval from an approver in each of these files:

    Approvers can indicate their approval by writing /approve in a comment
    Approvers can cancel approval by writing /approve cancel in a comment

    @ti-chi-bot ti-chi-bot bot added the lgtm label Jul 3, 2024
    Copy link

    ti-chi-bot bot commented Jul 3, 2024

    [LGTM Timeline notifier]

    Timeline:

    • 2024-07-03 05:47:36.675873902 +0000 UTC m=+1389783.161362734: ☑️ agreed by wuhuizuo.
    • 2024-07-03 05:48:49.937128452 +0000 UTC m=+1389856.422617284: ✖️🔁 reset by wuhuizuo.
    • 2024-07-03 05:49:50.673989086 +0000 UTC m=+1389917.159477918: ☑️ agreed by wuhuizuo.

    @ti-chi-bot ti-chi-bot bot merged commit e9f5fd4 into PingCAP-QE:main Jul 3, 2024
    2 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    2 participants