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

[libpng18] fix make check/test #596

Closed
wants to merge 1 commit into from

Conversation

jbowler
Copy link
Contributor

@jbowler jbowler commented Sep 15, 2024

This corrects the checks to that libpng 10800 does not turn on the
enhanced transform checks in either pngvalid or pngstest. The correct
fix is to change the 10700 code for comments which explain what aspect
or aspects of the transforms are broken (ideally) or at least state that
the transforms are broken.

@jbowler jbowler changed the base branch from libpng16 to libpng18 September 15, 2024 17:48
This corrects the checks to that libpng 10800 does not turn on the
enhanced transform checks in either pngvalid or pngstest.  The correct
fix is to change the 10700 code for comments which explain what aspect
or aspects of the transforms are broken (ideally) or at least state that
the transforms are broken.

Signed-off-by: John Bowler <[email protected]>
@jbowler jbowler force-pushed the pngvalid-test-breakage branch from 41da7d3 to f8dbbbd Compare September 18, 2024 19:46
@jbowler
Copy link
Contributor Author

jbowler commented Sep 18, 2024

Rebased on libpng18:HEAD, verified that "make check" still passes.

@ctruta
Copy link
Member

ctruta commented Sep 20, 2024

I integrated it in libpng18 and then I cherry-picked it in libpng16. ("Because I could.") Thank you.

@jbowler FYI, I'm trying to follow (admittedly not very strictly) commit naming rules like these:
https://www.conventionalcommits.org/en/v1.0.0/

I'm rewriting your subject lines if you noticed, and I hope you don't mind.

When I cherry-pick from libpng18 into libpng16, I'm adding the "[libpng16]" prefix, not entirely convinced that I should, but I'm doing it anyway for now. On the other hand, about the main development branch, I'd rather follow common practices used elsewhere, in which they make the subject lines short and sweet.

@ctruta ctruta closed this Sep 20, 2024
@jbowler
Copy link
Contributor Author

jbowler commented Sep 20, 2024

I'm trying to make the pull request titles meaningful in a list of pull requests, so they contain [branch] for example; information which is not relevant to the commit message, and SECURITY, which is probably not useful in a commit title either.

I limit the commit titles to the Linus-specified maximum length, which is really short, so they tend to come out missing English noise words, like is etc. I notice yours seems to be longer (it's been truncated in the header of the "CODE" tab) so I stop doing that.

@jbowler jbowler deleted the pngvalid-test-breakage branch September 21, 2024 01:03
@ctruta
Copy link
Member

ctruta commented Sep 25, 2024

If someone ever writes a book titled "The Subtle Art of Writing Commit Messages", I'll make sure to read it.

I "borrowed" the manner to write commit messages from various Google projects, which dates back from my very old days at Google. This, too, has been derived from a Google project, but I forgot which one.
https://www.conventionalcommits.org/en/v1.0.0/

I would like, myself, to keep the subject line short. Not really "Linus short" kind of short, but still, at least, it should be around 66 characters, if not shorter. I don't always succeed, but I still keep trying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants