Skip to content

Conversation

OskarStark
Copy link
Contributor

When multiple labels are added to a PR, ensure they are concatenated without spaces between brackets

@OskarStark OskarStark requested a review from xabbuh August 8, 2025 10:53
Added multiple test cases to ensure labels are always concatenated
without spaces between brackets. These tests verify the correct
behavior in various scenarios including incremental label additions
and empty titles.
@OskarStark OskarStark force-pushed the fix-label-spacing-in-titles branch from 24f7495 to 9c7dac8 Compare August 8, 2025 10:58
@OskarStark OskarStark removed the request for review from xabbuh August 8, 2025 10:58
@OskarStark OskarStark marked this pull request as draft August 8, 2025 10:58
This test was overly complex and didn't test anything that wasn't
already covered by other simpler tests.
@OskarStark
Copy link
Contributor Author

Hmm all the tests are passing

@xabbuh
Copy link
Member

xabbuh commented Aug 8, 2025

Do you have an example PR where this is not working as expected?

When a title contains bracketed text like [Foo] that isn't a recognized
label, and a real label is added, ensure all brackets are concatenated
without spaces. For example, adding 'Platform' label to '[Foo] Bar'
now produces '[Platform][Foo] Bar' instead of '[Platform] [Foo] Bar'.

- Extract and preserve bracketed text at the beginning of titles
- Concatenate all brackets (labels and non-labels) without spaces
- Add comprehensive test cases for mixed recognized/unrecognized brackets
@OskarStark
Copy link
Contributor Author

Yes, add the Platform label to this PR symfony/ai#282

While [OpenAi] is not a real label it is treated as title, but I would like to achieve this result:

- [Platform] [OpenAi] Add GPT 5 models
+ [Platform][OpenAi] Add GPT 5 models

@xabbuh
Copy link
Member

xabbuh commented Aug 8, 2025

I see, I thought this could be related to some kind of non-breaking space, but that doesn't seem to be the case.

@OskarStark OskarStark marked this pull request as ready for review August 8, 2025 11:56
Copy link
Member

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Thank you

Co-authored-by: Tobias Nyholm <tobias.nyholm@gmail.com>
@Nyholm
Copy link
Member

Nyholm commented Aug 9, 2025

Please fix the CS issues, then I would be happy to merge.

@OskarStark
Copy link
Contributor Author

Please fix the CS issues, then I would be happy to merge.

Done

@Nyholm Nyholm merged commit ed0ae68 into master Aug 9, 2025
4 checks passed
@Nyholm Nyholm deleted the fix-label-spacing-in-titles branch August 9, 2025 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants