Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Jun 16, 2025

No description provided.

llucax and others added 12 commits June 16, 2025 10:50
Signed-off-by: Leandro Lucarella <[email protected]>
When loading protobuf include paths from the pyproject.toml file, the
paths are received as an array/list. If we convert them to
a comma-separated string there is the risk of a path containing commas
to be split incorrectly, leading to compilation errors.

Using the original array/list of paths instead of a comma-separated
string is safer and more flexible.

Signed-off-by: Leandro Lucarella <[email protected]>
When include paths are passed via the commnad-line, they are passed as
a comma-separated string. This means the user could add extra spaces or
pass a double-comma, which would result in empty strings in the
include paths list, making the protobuf compiler fail.

This commit cleans the include paths by stripping whitespace and
ignoring empty strings.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This means that a version like v0x1e1 were accepted as valid semver
versions. Now this version is not considered a semver version anymore.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax added this to the v0.14.0 milestone Jun 16, 2025
@llucax llucax self-assigned this Jun 16, 2025
Copilot AI review requested due to automatic review settings June 16, 2025 13:02
@llucax llucax requested a review from a team as a code owner June 16, 2025 13:02
@llucax llucax added the type:tech-debt Improves the project without visible changes for users label Jun 16, 2025
@github-actions github-actions bot added part:tests Affects the unit, integration and performance (benchmarks) tests part:nox Affects the configuration of nox part:protobuf Affects the protobuf tools labels Jun 16, 2025
@llucax llucax enabled auto-merge June 16, 2025 13:02
@llucax llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Jun 16, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR merges v0.13.x into v0.x.x, updating tests, improving include_paths handling in the gRPC compile command, and fixing documentation typos and regex precision.

  • Adds new test cases for equality and malformed versions in compare_mike_version
  • Enhances CompileProto.run to accept both strings and iterables for include_paths
  • Corrects typos and tightens regex in mkdocs and nox configuration modules

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/mkdocs/test_mike.py Added test tuples for identical version strings and malformed version patterns
src/frequenz/repo/config/setuptools/grpc_tools.py Switched include_paths to `str
src/frequenz/repo/config/nox/init.py Fixed “typedefault” typo to “default”
src/frequenz/repo/config/mkdocs/mike.py Escaped dots in regex, tightened groupings, and added an exact-equality check
src/frequenz/repo/config/init.py Corrected “delelopment” typo to “development”

Comments suppressed due to low confidence (2)

tests/mkdocs/test_mike.py:269

  • Consider adding test cases for valid versions when both inputs are identical (e.g., ("v1.0-dev", "v1.0-dev", 0) and ("v1.0-pre", "v1.0-pre", 0)) to verify the new equality branch handles version strings as well.
        ("blah", "blah", 0),

src/frequenz/repo/config/setuptools/grpc_tools.py:85

  • Add unit tests for CompileProto.run() to cover both a comma-separated string and an Iterable input for include_paths, ensuring the match-case branches behave as expected.
        match self.include_paths:

@llucax llucax disabled auto-merge June 16, 2025 13:05
@llucax llucax enabled auto-merge June 16, 2025 13:08
@llucax
Copy link
Contributor Author

llucax commented Jun 17, 2025

Will force-merge, as this is just a merge...

@llucax llucax disabled auto-merge June 17, 2025 10:01
@llucax llucax merged commit 1a15f59 into v0.x.x Jun 17, 2025
27 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cmd:skip-release-notes It is not necessary to update release notes for this PR part:nox Affects the configuration of nox part:protobuf Affects the protobuf tools part:tests Affects the unit, integration and performance (benchmarks) tests type:tech-debt Improves the project without visible changes for users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants