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

Remove space between labels #14

Merged
merged 1 commit into from
Jul 16, 2024
Merged

Conversation

jaapio
Copy link
Contributor

@jaapio jaapio commented Jul 16, 2024

According to spec labels are separated by a comma, without any whitespace. To show this behavior we changed the integration test to have one case with 2 labels. This makes the integrationtest fail.

We are using the withLabels method twice because their is an incompatibility between php 7.3 and 7.4 vardict arguments are different ordered between those versions. By calling the method twice we can make sure the order is kept.

According to spec labels are separated by a comma, without any whitespace.
To show this behavior we changed the integration test to have one case
with 2 labels. This makes the integrationtest fail.

We are using the `withLabels` method twice because their is an incompatibility
between php 7.3 and 7.4 vardict arguments are different ordered between those versions.
By calling the method twice we can make sure the order is kept.
@jaapio jaapio requested a review from hollodotme as a code owner July 16, 2024 11:47
@hollodotme hollodotme self-assigned this Jul 16, 2024
@hollodotme hollodotme added the bug Something isn't working label Jul 16, 2024
@hollodotme hollodotme added this to the v0.4.1 milestone Jul 16, 2024
@hollodotme
Copy link
Member

@jaapio Thanks for spotting this. 💪 It's not that obvious in the spec.

I'll merge it and release v0.4.1.

@hollodotme hollodotme merged commit 8c77eba into openmetrics-php:master Jul 16, 2024
@hollodotme
Copy link
Member

✅ v0.4.1 was released.

@jaapio
Copy link
Contributor Author

jaapio commented Jul 16, 2024

Thanks for merging and releasing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants