Fix inverted expected/found FreeType version warning. #7881
Conversation
There was a problem hiding this comment.
The patch looks fine, but I would not tag this for 2.0.1
I am once again concerned about how many pull requests are being tagged for 2.0.1 and are backported. We have more pull request tagged 2.0.1 than we have for 2.1. At this point, it'll be easier to do the next minor release than the 2.0.1 micro release.
|
On this particular one, it's targetted to the right branch, so no backport for the 2.0.1 release--just a merge to master at some point. It's also clearly a bug fix (not super critical, but almost zero risk). I'm not sure what's wrong with putting this in the next release. |
|
@dopplershift My concern with this state of mind is that we backport everything. |
|
Ah. Might want to pick that battle on something that's more arguably out of bounds. 😁 |
|
@dopplershift I think only major bugs should be backported: this isn't a major bug. |
|
My take is if it's a bug, and the fix is ready, it goes in, with one proviso: The lines in question have to have tests to ensure that it didn't accidentally break something. It doesn't have to be a new test (a test for this specific change would just be silly), but the test suite should be executing any lines that changed. I have no idea if this change was run by the tests or not. |
2de8e80 to
ec92ff5
Compare
Unfortunately not; all tests are run with the builtin freetype version, so this warning will never be triggered on CI. Locally, however, I never build with |
|
Less concerned about whether they're backwards (I trust you) and more about making sure there's a test somehow that makes sure there's not an unexpected error lurking in the change--if we're planning on putting this in 2.0.1. |
|
Merged, but I'm not entering the debate of whether to backport or not :) |
|
Apparently this went into 2.0.x, not master (I just clicked the green button). Someone please check whether this was correct... |
I've always been confused by this message, and it turns out it's backwards.