search

RefactoringCombos / ArlosCommitNotation

A notation for small commits messages that show the risk involved in each step
321 stars 30 forks source link

It's OK to modify comments, too. #22

Closed JayBazuzi closed 3 years ago

arlobelshee commented 4 years ago

Do we even need c? How does this differ from d?

JayBazuzi commented 4 years ago

It depends. On context.

Editing README.md / TODO.md / CONTRIBUTING.md feels different than editing a comment in code.

In Python I can annotate types with comments:

def factorial(i): 
   # type: (int) -> int

Editing this won't change the behavior of the script but can cause new lint errors which may or may not break the build or deployment pipline. How do we want to represent that kind of risk?

If I'm using MarkdownSnippets then changing a doc can affect the build.

Llewellyn doesn't see editing comments as "safe" without additional checks: he compares the size of the compiled output binary before and after the comment edit. He says he has been burned when he thought he was only editing comments but got carried away and edited something else.

I'm not arguing for one position or another, just trying to make things more confusing.

arlobelshee commented 3 years ago

In general, I'm trying to limit the number of different letters. I'm doing this for 2 reasons:

  1. My current company is using this notation in order to improve regulatory review. Fewer categories means less nuance and makes review easier - and improves consistency across the org. I would love for more teams to be able to simplify their regulatory context.
  2. Many users (including me) extend this with custom actions that match their domain. For example, when I'm making a website, then I need to add letters for new copy, and for editing. I want to leave lots of letters open for this kind of local extension.
arlobelshee commented 3 years ago

That said, I'm more open to extending an existing letter to show up in a different risk category. Perhaps you've found a legitimate case for D?

d -> dev-facing docs that do not share files with source, or done in a safe way (Llewellyn's comment edit safety check) D -> dev-facing docs that share code files (Llewellyn's comment example, without the safety check) D!! -> dev-facing docs that could be dangerous (like changing the types if you can't test that without a commit)

arlobelshee commented 3 years ago

Does the current proposal for splitting risk from intention address this concern? If so, can we close the PR? If not, can you modify that PR to meet this need?

JayBazuzi commented 3 years ago

Sounds good to me.