- Issue created by @niklan
- last update
over 1 year ago 29,438 pass, 1 fail - 🇬🇧United Kingdom catch
That is probably a bug in Peast https://github.com/mck89/peast
- Status changed to Needs review
over 1 year ago 1:56pm 15 July 2023 - last update
over 1 year ago 29,446 pass - 🇷🇺Russia niklan Russia, Perm
Seems like it was fixed and available in Peast 1.15.2 release. So I think we have to update this issue to reflect that.
Not sure if we should keep provided test, because it tests third-party library, actually.
P.s. I'm not certain about bumping minimal version, but it sounds reasonable, because otherwise it can break aggregation.
- 🇫🇷France andypost
Can you add a unit test to prevent regressions in future, that's a bug
- 🇷🇺Russia Chi
I think there is no need to create tests for third party packages. If this functionality is critical it's better to submit the test to upstream project.
- Status changed to RTBC
over 1 year ago 3:04pm 15 July 2023 - 🇳🇱Netherlands spokje
I agree with @Chi on this one (sorry @andypost), let's see what core committers think.
- 🇬🇧United Kingdom catch
Peast added their own test coverage so I think it's very unlikely to regress.
What we could maybe do if we wanted to would be to change the content of another existing test file for that test class, to include the syntax here (since they don't have any particular syntax in, just whitespace to strip) - then we'd be covering this bug without actually adding more tests to maintain overall.
I will aim to commit this early next week either way so that it definitely lands in the next patch release. Very good find and also extremely encouraging that Peast fixed it within half a day of us opening the issue.
- 🇳🇱Netherlands spokje
@catch: Could it be that this issue, due to the new non-auto-checkbox-for-credit-checking on d.o. doesn't have the credits which you would expect from reading the commit message?
I really don't need the credit, but I feel at the very least Niklan should get one for reporting the issue here, writing a fail test and commenting on the peast issue.
- Status changed to Fixed
over 1 year ago 9:01am 17 July 2023 - 🇬🇧United Kingdom catch
The MR diff didn't cleanly apply to 11.x, I also noticed that it increased the minimum Peast version in composer.json.
So I did the update locally - Peast still updated to the latest version, but without the composer.json change.
Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!
Automatically closed - issue fixed for 2 weeks with no activity.
