BIP78: spelling and grammar updates #1623
Conversation
bip-0078.mediawiki
Outdated
| @@ -189,7 +189,7 @@ The well-known error codes are: | |||
|
|
|||
| The receiver is allowed to return implementation specific errors which may assist the sender to diagnose any issue. | |||
|
|
|||
| However, it is important that error codes that are not well-known and that the message do not appear on the sender's software user interface. | |||
| However, it is important that error codes are not well-known and that the messages do not appear on the sender's software user interface. | |||
There was a problem hiding this comment.
I'm not sure that this is the intended meaning (see the lines immediately following this one). It looks to me like the idea is that error codes that are not well known should not appear in the UI and be printed in the debug log only. Also, this change would require author sign-off to ensure it doesn't alter the intended meaning of the text.
There was a problem hiding this comment.
Every spec I've seen has separate well-known error codes with templates to display to users, and a generic unknown error code for which messages they contain are only printed to debug logs. I missed that this changed themeaning in my first review. It should be reworded not to change the meaning in my view.
There was a problem hiding this comment.
suggestion: However, it is important that error codes and messages that are not well-known, do not appear on the sender's software user interface.
There was a problem hiding this comment.
Thanks @remcoros! I like that suggestion. I think it brings more clarity while still preserving the meaning. Admittedly, this sentence has been a bit of a head scratcher for me. If I adopt remcoros's suggestion, I don't think it would it need BIP author sign off, right?
For now I rebased and just left this sentence alone. If there is consensus on how to reword it without changing the meaning, I am happy to open up a follow up PR.
There was a problem hiding this comment.
@satsie Happy to review your future follow-up to reword this confusing passage in a way that makes sense without changing the intended meaning.
murchandamus
added
Proposed BIP modification
Pending acceptance
|
Thanks for the review @jonatack! I will create a separate PR with the changes that are definitely just spelling or grammar related and rebase this (since it has comment history) to only include the ones that would need sign off from the BIP author. |
|
@satsie following up on the thread #1623 (comment), perhaps keep all the changes in this PR if while updating your change there it becomes clear that they are editorial-only and don't alter the meaning of the text. I'm not sure the BIP author is active in providing review feedback here at the moment. |
satsie
force-pushed
the
satsie-bip78
branch
2 times, most recently
from
22610cd
to
b3445ea
Compare
Co-authored-by: Dan Gould <[email protected]> Co-authored-by: Jon Atack <[email protected]>
Oh that is a good catch! I accepted the suggestion and also took the advice to replace "url" with "URL". |
This PR proposes a number of spelling and grammar updates to BIP78. Nothing here should change the actual content of the BIP, though I'm new to Payjoin and would appreciate a sanity check :)
cc the BIP champion, @NicolasDorier