autowarefoundation / autoware-github-actions

Apache License 2.0
17 stars 24 forks source link

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

Closed sasakisasaki closed 1 month ago

sasakisasaki commented 2 months ago

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:

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](https://github.com/autowarefoundation/autoware_dummy_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](https://github.com/sasakisasaki/autoware-github-actions/tree/fix-regular-expression-pattern-for-semantic-versioning/create-prs-to-update-vcs-repositories#inputs) 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.

- [x] I've confirmed the [contribution guidelines].
- [x] The PR follows the [pull request guidelines].

## 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.

- [x] 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.

[contribution guidelines]: https://autowarefoundation.github.io/autoware-documentation/main/contributing/
[pull request guidelines]: https://autowarefoundation.github.io/autoware-documentation/main/contributing/pull-request-guidelines/
sasakisasaki commented 2 months ago

This PR is now under testing.

sasakisasaki commented 2 months ago

Now it is ready for review :+1:

mitsudome-r commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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! :+1:

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! :+1:

mitsudome-r commented 2 months ago

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.

sasakisasaki commented 2 months ago

@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 commented 2 months ago

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

xmfcx commented 2 months ago

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

sasakisasaki commented 2 months ago

@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.

sasakisasaki commented 1 month ago

@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 commented 1 month ago

(CC: @youtalk)