Skip to content

MAINTAINING.md: Add entry on exceptions#12785

Closed
maribu wants to merge 3 commits intoRIOT-OS:masterfrom
maribu:common_sense
Closed

MAINTAINING.md: Add entry on exceptions#12785
maribu wants to merge 3 commits intoRIOT-OS:masterfrom
maribu:common_sense

Conversation

@maribu
Copy link
Copy Markdown
Member

@maribu maribu commented Nov 22, 2019

Contribution description

Clarify that common sense should always be applied and that sometimes exceptions from our guidelines are preferable over blindly applying rules.

Testing procedure

Check the changes in regard to its content, grammar and spelling.

Issues/PRs references

In #12781 the fear was expressed that a section of the coding convention might be applied blindly. This tries to raise awareness of that issue in general.

Clarify that common sense should always be applied and that sometimes exceptions
from our guidelines are preferable over blindly applying rules.
@maribu maribu added Discussion: needs consensus Marks a discussion that needs a (not necessarily moderated) consensus Process: needs >1 ACK Integration Process: This PR requires more than one ACK Area: doc Area: Documentation labels Nov 22, 2019
@maribu maribu requested a review from a team November 22, 2019 16:46
@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 22, 2019

This somewhat repeats partly what is already stated in the introduction:

  • Any of the items in this list might be skipped, if clearly and logically
    articulated reasons are presented.

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 22, 2019

How about "can be presented" instead of "are presented"? Otherwise it sounds like those reasons have to be documented. In cases it is not obvious why an exception was made, documenting it might make sense. But e.g. for exceptions to the coding convention I'd personally wouldn't like to "fill out forms to formally grant an exception" (purposely wildly exaggerating here).

@maribu
Copy link
Copy Markdown
Member Author

maribu commented Nov 22, 2019

@miri64: I deduplicated the section you quoted from the introduction and reused its wording.

The diff in content are now:

  • The exceptions are broader to apply on all guidelines and explicitly also on the coding convention
  • The need to document the reasons for the exception is weakened to can be presented

@waehlisch
Copy link
Copy Markdown
Member

i'm not in favor of the proposed changes. (1) a separate section unnecessarily blows up the document. the statement is already prominently located (first bullet point). in particular, it is located up-front of Section 4. - Review the code against the coding conventions, which means it also applies to Section 4. (2) There is no reason to weaken the statement. usually, it is important to document exceptions. the description in the Coding Conventions is sufficiently flexible. ("shall" instead of "must")

@stale
Copy link
Copy Markdown

stale bot commented May 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

@stale stale bot added the State: stale State: The issue / PR has no activity for >185 days label May 26, 2020
@stale stale bot closed this Jun 26, 2020
@maribu maribu deleted the common_sense branch January 25, 2021 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: doc Area: Documentation Discussion: needs consensus Marks a discussion that needs a (not necessarily moderated) consensus Process: needs >1 ACK Integration Process: This PR requires more than one ACK State: stale State: The issue / PR has no activity for >185 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants