Skip to content

unit1612: Fixed the compilation of the new unit test#5024

Closed
captain-caveman2k wants to merge 1 commit intocurl:masterfrom
captain-caveman2k:unit1612
Closed

unit1612: Fixed the compilation of the new unit test#5024
captain-caveman2k wants to merge 1 commit intocurl:masterfrom
captain-caveman2k:unit1612

Conversation

@captain-caveman2k
Copy link
Contributor

Follow on from 3f74e5e. Due to a typo in Makefile.inc unit1611 was used as test1612. Once corrected there were a few minor compilation issues to fix.

@jay
Copy link
Member

jay commented Mar 3, 2020

unit1612: Fix the compilation of the new unit test

@bagder
Copy link
Member

bagder commented Mar 3, 2020

if we'll be very strict, I would also prefer if we dropped the initial uppercasing of the first letter and when possible rather spell out the actual fix rather than saying "fix". Perhaps:

unit1612: compile the correct source file for the test

@jay
Copy link
Member

jay commented Mar 3, 2020

I think as a requirement it's probably too strict. Keywords like fix/add/remove or whatever may help communicate an overall effect (which is then fully spelled out in the commit message), and the initial uppercase style can be found in the history occasionally. I have some bias. Take for example

curl_getenv.3: Fix the memory handling description

- Tell the user to call curl_free() to free the pointer returned by
  curl_getenv().

It could be done with some limitation in the subject as

curl_getenv.3: tell the user to call curl_free() to free the pointer

I think the first one communicates better. I'm not sure if everyone shares that view.

@bagder
Copy link
Member

bagder commented Mar 3, 2020

I didn't mean it as a requirement. I just offered my take on what would make the commit message even better in my view.

Follow on from 3f74e5e. Due to a typo in Makefile.inc unit1611 was
used as test1612. Once corrected there were a few minor compilation
issues to fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants