mild approve()/Approval() clarifications

[shrugs]

In ERC721, specify that Approval should be emitted in a "set or reaffirmation" and not just on a "change or reaffirmation".

[shrugs]

cc @fulldecent for author approval

[fulldecent]

@shrugs For @notice lines please do not use trailing period. Also, please fix the other one in there.

FYI I got this convention from https://github.com/ethereum/wiki/wiki/Ethereum-Natural-Specification-Format

[shrugs]

that natspec spec doesn't really specify whether or not periods are required or not, including giving multiple contradictory examples and unspecified multiline specifications

/// @notice (balanceInmGAV / 1000).fixed(0,3) GAV is the total funds available to who.address().

Also in this spec document are

/// @notice Enable or disable approval for a third party ("operator") to manage
/// all of msg.sender's assets.

and

/// @notice A distinct Uniform Resource Identifier (URI) for a given asset.

along with inconsistent periods at the end of @param tags like

/// @param _operator Address to add to the set of authorized operators.

Is the rule we want to follow "no trailing periods except on @dev tags?" If so, I can update the spec to conform to that.

[mg6maciej]

Shouldn't it say "set, changed or reaffirmed"?

[fulldecent]

Yes, that makes sense. Please use "set, changed or reaffirmed"

Also welcome to make any period fixes too.

Thank you.

[mg6maciej]

Looking at it again and at 0xcert and openzeppelin implementations, the Approval event seems to be emitted in call cases:

• set (none -> address)
• unset (address -> none)
• changed (address -> another address)
• reaffirmed (none -> none or address -> same address)

"Changed" can also be understood as a set of {set, unset, changed}, so I think it might be better to leave it as "changed or reaffirmed" instead of listing all cases or maybe just write something along the lines of "In all cases Approval event is emitted".

[fulldecent]

"Changed or reaffirmed" is good