Create developer-notes.md and add ack/nack explanations #5468

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:developer-notes_md changing 3 files +50 −37

fanquake commented at 4:43 AM on December 13, 2014: member

Following up on ACK/NACK discussion on the mailing list. Couldn't figure out where the best place to add documentation for pull request terms would be. Decided that creating a developer notes file and combining the existing notes/guidelines could be beneficial.

Moves coding guidelines and development tips/tricks into a single file. Also adds a section explaining ack/nack.
jonasschnelli commented at 7:14 AM on December 13, 2014: contributor

ACK

The filename developer-notes.md could also be something like guidelines-for-developers (or similar). Because developer-notes could also imply that this are notes from the developers. But english is not my main-play.
fanquake force-pushed on Dec 15, 2014
laanwj added the label Docs and Output on Dec 16, 2014
btcdrak commented at 8:47 AM on December 17, 2014: contributor

nit: missing info about nits :speak_no_evil:
in doc/developer-notes.md.md:None in 3a578c6e03 outdated
```
 177 | +
 178 | +Concept ACK - Agree with the idea and overall dircetion, but haven't reviewed the code changes or tested them.
 179 | +
 180 | +utACK (untested ACK) - Reviewed and agree with the code changes but haven't actually tested them.
 181 | +
 182 | +Tested ACK - Reviewed the code changes and have verified the functionality or bug fix.
```
Diapolo commented at 8:56 AM on December 17, 2014:

Is a Tested ACK not just an ACK :)?

laanwj commented at 9:00 AM on December 17, 2014:

No, a loose ACK is ambigious. Avoid them unless it's a documentation/comment only change in which case there is nothing to test so the tested/untested distinction is not there.

Diapolo commented at 9:01 AM on December 17, 2014:

Understood!
fanquake force-pushed on Dec 17, 2014
fanquake commented at 11:08 AM on December 17, 2014: member

Fixed a typo, and added @laanwj's explanation of ACK.
sipa commented at 11:51 AM on December 17, 2014: member

I typically use ACK these days to mean "Untested ACK, but I don't think this requires testing". Maybe I'm alone :)
laanwj commented at 12:22 PM on December 17, 2014: member

@sipa That's the same place I use them, i.e. comments and documentation changes are prime examples.
Michagogo commented at 10:27 PM on December 17, 2014: contributor

Uh, check your file name... You seem to have a double extension.

Create developer-notes.md

Moves coding guidelines and development tips/tricks into a single file.
Also adds a section explaining pull request terminology.

3bf5f52808

fanquake force-pushed on Dec 19, 2014
fanquake commented at 3:16 AM on December 19, 2014: member

Have fixed the double extension, thanks @Michagogo
in doc/developer-notes.md:None in 3bf5f52808
```
 181 | +
 182 | +Tested ACK - Reviewed the code changes and have verified the functionality or bug fix.
 183 | +
 184 | +ACK -  A loose ACK can be confusing. It's best to avoid them unless it's a documentation/comment only change in which case there is nothing to test/verify; therefore the tested/untested distinction is not there.
 185 | +
 186 | +NACK - Disagree with the code changes/concept. Should be accompanied by an explanation. 
```
btcdrak commented at 6:35 AM on December 19, 2014:

Please add a sentence about "nits".

laanwj commented at 6:56 AM on December 19, 2014:

I nit is simply a small comment. It comes from 'nitpicking' which is English, which means 'to be excessively concerned with or critical of inconsequential details'. It's a kind of self-criticizing at time same time. 'I like it slightly better like this but it isn't really important'... It's a kind of silly colloquialism and doesn't belong in this 'formal' list.
jgarzik merged this on Dec 31, 2014
jgarzik closed this on Dec 31, 2014
jgarzik referenced this in commit ccef2b5f26 on Dec 31, 2014
jgarzik commented at 12:44 PM on December 31, 2014: contributor

ACK
fanquake deleted the branch on May 12, 2016
DrahtBot locked this on Sep 8, 2021

Contributors

Labels