search

nsf / termbox

Library for writing text-based user interfaces
http://code.google.com/p/termbox
MIT License
1.96k stars 185 forks source link

add sys/select.h to includes #99

Closed dcat closed 7 years ago

dcat commented 7 years ago

add sys/select.h to includes

rofl0r commented 7 years ago

your commit message describes what you're doing, but not why you're doing it. missing critical info.

dcat commented 7 years ago

well... because termbox.c uses select().

rofl0r commented 7 years ago

it's still missing in the commit message. (hint: some ppl try to figure out what happened in a repo by reading commit messages. in your case they have the look at the code additionally to figure out why)

dcat commented 7 years ago

rebased the commit message to be single-line, hopefully now you'll be able to read it

rofl0r commented 7 years ago

the problem is not that it wasn't single line (albeit the first line "update foo.c" was a bit too generic). it's that it's missing the rationale, still. so a good commit message would tell a good summary what it's doing in the first line, and then go on to explain the changes and the rationale for the changes in the next lines.

dcat commented 7 years ago

wait, you want me to explain the rationale between adding a dependency?

rofl0r commented 7 years ago

i don't want you to do anything, since this is not my repo. however if it was mine, i'd request a commit message explaining the "why". for example:

foo.c: add missing include sys/select.h

the code uses select(), but the header defining it was not included.
this broke compilation on AIX.
dcat commented 7 years ago

well, let's hope the author isn't as autisticly pedantic as you are.