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

fix(create-prs-to-update-vcs-repositories): update only for versions whose format is X.Y.Z #320

Conversation

sasakisasaki
Copy link
Contributor

@sasakisasaki sasakisasaki commented Sep 12, 2024

Description

Create PRs to update a version of vcs-imported repository in the autoware.repos if its version format matches for example "0.0.1", "1.0.1", "1.2.3", ... etc.

Related links

Semantic versioning discussion
Discussion on the revert PR

Tests performed

Ongoing in this repository. As the tests done, the detail will be described here.

In this PR, I focused on testing if only the expected regex patterns are handled as we already tested the basic features in this PR.

I have tested as following steps:

  • Prepare a sample python script as follows to check if the regex works only for the expeced version strings
import re

# The regular expression pattern to match specific semantic versions
pattern = r'\b(?<![^\s])\d+\.\d+\.\d+(?![-\w.+])\b'

# List of example patterns to test
examples = [
    "0.0.1",                # Should match
    "0.1.0",                # Should match
    "1.0.0",                # Should match
    "2.1.1",                # Should match
    "v0.0.1",               # Should NOT match
    "ros2-v0.0.4",          # Should NOT match
    "xxx-1.0.0-yyy",        # Should NOT match
    "v1.2.3-beta",          # Should NOT match
    "v1.0",                 # Should NOT match
    "v2",                   # Should NOT match
    "1.0.0-alpha+001",      # Should NOT match
    "v1.0.0-rc1+build.1",   # Should NOT match
    "2.0.0+build.1848",     # Should NOT match
    "2.0.1-alpha.1227",     # Should NOT match
    "1.0.0-alpha.beta",     # Should NOT match
    "ros_humble-v0.10.2"    # Should NOT match
]

# Function to check if the version matches the pattern
def check_version(version):
    return bool(re.fullmatch(pattern, version))

# Check and print results
for example in examples:
    result = check_version(example)
    print(f"Version: {example} -> Match: {result}")
  • Applied the changes as this PR
  • Tested this PR's workflow from this repository with changing versions in the autoware.repos
    • Behavior is as expected: PR is created only for expected format versions

Notes for reviewers

Let me describe following two stuffs.

What kind of tags are handled

  • Monitors all vcs-imported repositories in the autoware.repos which have a version with regular expression pattern r'\b(?<![^\s])\d+\.\d+\.\d+(?![-\w.+])\b' (if default).
    • This pattern match/mismatches for the following examples:
        "0.0.1",                # match
        "0.1.0",                # match
        "1.0.0",                # match
        "2.1.1",                # match
        "v0.0.1",               # mismatch
        "ros2-v0.0.4",          # mismatch
        "xxx-1.0.0-yyy",        # mismatch
        "v1.2.3-beta",          # mismatch
        "v1.0",                 # mismatch
        "v2",                   # mismatch
        "1.0.0-alpha+001",      # mismatch
        "v1.0.0-rc1+build.1",   # mismatch
        "2.0.0+build.1848",     # mismatch
        "2.0.1-alpha.1227",     # mismatch
        "1.0.0-alpha.beta",     # mismatch
        "ros_humble-v0.10.2"    # mismatch

What kind of version update is possible?

  • If there is a new version with pattern matched in the vcs-imported repositories, create a PR for each repository, respectively.
  • The valid/invalid version update cases are as follows:
    • Valid ones (PR must be created):
    0.0.1  =>  0.0.2
    1.1.1  =>  1.2.1
    2.0.9  =>  3.1.4

Note that we can specify what kind of version update must be handled.
Please see this README.md for more details.

  • Invalid ones (PR is not created):
    main       =>  0.0.1
    v0.0.1     =>  0.0.2
    xxx-0.0.1  =>  0.0.9

Interface changes

No interface change

ROS Topic Changes

No ROS topic change

ROS Parameter Changes

No ROS parameter change

Effects on system behavior

As the autowarefoundation/autoware start using this workflow, new PRs will be created automatically on the autowarefoundation/autoware as the vcs-repository has the version update

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

  * Only handles formats such as: "1.0.0", "1.1.1", "0.0.9", ... etc
  * Following ones are the mismatched examples:
```
        "v0.0.1",               # mismatch
        "ros2-v0.0.4",          # mismatch
        "xxx-1.0.0-yyy",        # mismatch
        "v1.2.3-beta",          # mismatch
        "v1.0",                 # mismatch
        "v2",                   # mismatch
        "1.0.0-alpha+001",      # mismatch
        "v1.0.0-rc1+build.1",   # mismatch
        "2.0.0+build.1848",     # mismatch
        "2.0.1-alpha.1227",     # mismatch
        "1.0.0-alpha.beta",     # mismatch
        "ros_humble-v0.10.2"    # mismatch
```

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki
Copy link
Contributor Author

This PR is now under testing.

sasakisasaki added a commit to autowarefoundation/autoware_dummy_repository that referenced this pull request Sep 12, 2024
  * For testing a functionality in the following PR:
    - autowarefoundation/autoware-github-actions#320

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki sasakisasaki changed the title fix(create-prs-to-update-vcs-repositories): fix as follows (see below) fix(create-prs-to-update-vcs-repositories): Handle only r'\b(?<![^\s])\d+\.\d+\.\d+(?![-\w.+])\b' Sep 12, 2024
@sasakisasaki sasakisasaki changed the title fix(create-prs-to-update-vcs-repositories): Handle only r'\b(?<![^\s])\d+\.\d+\.\d+(?![-\w.+])\b' fix(create-prs-to-update-vcs-repositories): Update only for versions whose format is X.Y.Z Sep 12, 2024
@sasakisasaki sasakisasaki changed the title fix(create-prs-to-update-vcs-repositories): Update only for versions whose format is X.Y.Z fix(create-prs-to-update-vcs-repositories): update only for versions whose format is X.Y.Z Sep 12, 2024
@sasakisasaki sasakisasaki marked this pull request as ready for review September 12, 2024 11:36
@sasakisasaki
Copy link
Contributor Author

Now it is ready for review 👍

@mitsudome-r
Copy link
Member

Valid ones (PR must be created):
0.0.1 => 0.0.2
1.1.1 => 1.2.1
2.0.9 => 3.1.4

Do we want to have PRs created to bump major version like for 2.0.9 -> 3.1.4?
I was thinking there could be a situation where we might want to keep the major version for some compatibility reasons, and it might be annoying to have the action keep creating PRs for 3.x.x.

@mitsudome-r
Copy link
Member

Is it possible for you to update the README to add "What kind of tags are handled?" and "What kind of version update is possible?" that you explained in your PR description?

@sasakisasaki
Copy link
Contributor Author

@mitsudome-r Thank you very much for the informative comments! Let me answer for each:

Do we want to have PRs created to bump major version like for 2.0.9 -> 3.1.4?
I was thinking there could be a situation where we might want to keep the major version for some compatibility reasons, and it might be annoying to have the action keep creating PRs for 3.x.x.

Informative question. I want to have a feedback from @youtalk to understand what is the desired version transition.
On the other hand, remembering that the PR creation happens every 6 hours, the transition like 2.0.9 => 3.1.4 hardly happen (my provided example was bad, sorry). Because the version update does not happen multiple times in 6 hours. Considering 6 hours interval check, I think the version update will be performed as like: 2.0.9 => 3.0.0 => 3.0.1 => ... etc.

Hopefully this provides the answer otherwise please feel free to share your ideas and so on! 👍

Is it possible for you to update the README to add "What kind of tags are handled?" and "What kind of version update is possible?" that you explained in your PR description?

Sure, let me update the README.md! 👍

sasakisasaki and others added 8 commits September 13, 2024 11:54
  * Make clear what is the condition for creating a PR

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…ed code block

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
… of https://github.com/sasakisasaki/autoware-github-actions into fix-regular-expression-pattern-for-semantic-versioning

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
… of https://github.com/sasakisasaki/autoware-github-actions into fix-regular-expression-pattern-for-semantic-versioning

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…ation

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@mitsudome-r
Copy link
Member

mitsudome-r commented Sep 13, 2024

Thanks for updating the README.

On the other hand, remembering that the PR creation happens every 6 hours, the transition like 2.0.9 => 3.1.4 hardly happen (my provided example was bad, sorry). Because the version update does not happen multiple times in 6 hours. Considering 6 hours interval check, I think the version update will be performed as like: 2.0.9 => 3.0.0 => 3.0.1 => ... etc.

I was a more concerned that there could be a situation where 2.x.x. could be updated after 3.0.0 is released.

Consider the following situation for example:

  1. We currently have ROS Humble version of Autoware which depends on autoware_msgs version 1.1.0
  2. Let's say we decide to transition Autoware to ROS Jazzy, and we released autoware_msgs with version 2.0.0 in ROS Jazzy, which is incompatible with the older version
  3. During the transition period, we would like to continue support of Humble version of Autoware for certain period of time(e.g., for 6 month) to give enough time for the users. Therefore, there would be a humble branch autoware that depends on autoware_msgs 1.1.0 and jazzy branch that depends on autoware_msgs 2.0.0.
  4. During the transition period, we noticed a bug in autoware_msgs 1.1.0 and decided to release the fixed version as version 1.2.0.

In this case, we don't want this GitHub Action to create a PR to humble branch to update to version 2.0.0, but only create a PR for 1.2.0.

pre-commit-ci bot and others added 2 commits September 13, 2024 18:41
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Co-authored-by: Ryohsuke Mitsudome <43976834+mitsudome-r@users.noreply.github.com>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki sasakisasaki force-pushed the fix-regular-expression-pattern-for-semantic-versioning branch from b2287e4 to 6694ec4 Compare September 13, 2024 09:42
@sasakisasaki
Copy link
Contributor Author

sasakisasaki commented Sep 17, 2024

@mitsudome-r Thank you for sharing that possible case. Only the major update (e.g. 1.x.y => 2.x'.y') can be done by manually, and the other minor updates (e.g. x.a.b => x.a'.b') are handled by this workflow looks make sense.

@youtalk @xmfcx How about the idea above? Thanks!

@sasakisasaki
Copy link
Contributor Author

Perhaps adding option like --create-prs-for-major-updates (default=False) is one of the possible solutions.

@xmfcx
Copy link
Contributor

xmfcx commented Sep 17, 2024

Let's create different prs for major prs and minor prs separately from the same workflow.

@sasakisasaki
Copy link
Contributor Author

@mitsudome-r @xmfcx (CC: @youtalk ) As we discussed in the Software Working Group, I'll apply the fix so we create the PR for both major and minor ones, respectively.

  * Do not allow to change the semver pattern via argument: use only fixed pattern.
  * Add description to make it explicit what kind of patterns are supported.

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…ee below):

  * Support separated PRs for major, minor, and patch updates
  * If not specified, PR will be created for any updates

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
…" from spell check

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
sasakisasaki added a commit to autowarefoundation/autoware_dummy_repository that referenced this pull request Sep 26, 2024
  * For testing this PR:
      autowarefoundation/autoware-github-actions#320

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
  * The existing branch list is not updated correctly.
    When major, minor, patch, and any are all enabled,
    The same name branch can be processed twice.
    This eventually causes an error like:
```
nothing to commit
```

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki
Copy link
Contributor Author

sasakisasaki commented Sep 30, 2024

@mitsudome-r @xmfcx I think if the PRs are created as this, this PR perhaps satisfies the requirements. If this matches to your understanding, I think we can use this PR for vcs repository's versioning.

(EDITTED): We can specify which updates the PR should be created for (see this README.md for more details).

@sasakisasaki
Copy link
Contributor Author

(CC: @youtalk)

  * The information is duplicated that of README.md

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
  * Keep consistent indentation

Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM! Congrats!

@sasakisasaki sasakisasaki enabled auto-merge (squash) October 1, 2024 08:39
@sasakisasaki sasakisasaki disabled auto-merge October 1, 2024 09:00
Signed-off-by: Junya Sasaki <junya.sasaki@tier4.jp>
@sasakisasaki sasakisasaki merged commit e1102ae into autowarefoundation:main Oct 1, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants