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

Make modifiable entity groups greedy #286

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Conversation

nforro
Copy link
Member

@nforro nforro commented Sep 18, 2023

This effectively makes the first group most greedy, rather than the last. So, with the following in a spec file:

%global apiver 2.15
%global patchver 3

Version: %{apiver}.%{patchver}

calling spec.update_tag("Version", "2.16.0"), the apiver macro gets assigned the bigger portion of the version string, resulting in:

%global apiver 2.16
%global patchver 0

rather than:

%global apiver 2
%global patchver 16.0

There are cases (in Fedora spec files) where the previous logic makes more sense, but only a handful of them, and none of them using packit.
With the current implementation, it's one or the other, and I believe this makes more sense.

Related to packit/packit#2033

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/af0cd4f35e3047e482909ba76d398257

✔️ pre-commit SUCCESS in 1m 23s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 02s
✔️ specfile-tests-pip-deps SUCCESS in 1m 00s

Copy link
Member

@lachmanfrantisek lachmanfrantisek left a comment

Choose a reason for hiding this comment

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

Can you please add a new test case for this?

This effectively makes the first group most greedy, rather than the
last. So, with the following in a spec file:

```
%global apiver 2.15
%global patchver 3

Version: %{apiver}.%{patchver}
```

calling `spec.update_tag("Version", "2.16.0")`, the `apiver` macro
gets assigned the bigger portion of the version string, resulting in:

```
%global apiver 2.16
%global patchver 0
```

rather than:

```
%global apiver 2
%global patchver 16.0
```

There are cases (in Fedora spec files) where the previous logic
makes more sense, but only a handful of them, and none of them
using packit.
With the current implementation, it's one or the other, and I believe
this makes more sense.

Signed-off-by: Nikola Forró <[email protected]>
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/550baaebda6346409112c3d4a92f5c3f

✔️ pre-commit SUCCESS in 1m 28s
✔️ specfile-tests-rpm-deps SUCCESS in 1m 05s
✔️ specfile-tests-pip-deps SUCCESS in 1m 04s

@nforro nforro added the mergeit Zuul, merge it! label Sep 19, 2023
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).
https://softwarefactory-project.io/zuul/t/packit-service/buildset/f9ac96c78cde48eca1e9392f06e7ba3f

✔️ pre-commit SUCCESS in 1m 32s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 4a8aadf into main Sep 19, 2023
@delete-merged-branch delete-merged-branch bot deleted the update-tag branch September 19, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit Zuul, merge it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants