Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I don't want you to write an essay; I want to point out, that there is a third state, besides block and approve. I don't want to never have that code in case, the author disagrees with me or I am missing something.


Teams I have worked in often have an Approved subject to comments” with th “all comments must be resolved” marker set.

This somewhat violates the articles “approval or not is the only bit that matters” argument, but it at least forces participants to at least acknowledge an issue, and either provide a satisfactory reason to resolve on the spot, or open a new ticket for that work.


That was addressed by someone else: https://news.ycombinator.com/item?id=45705759 . Also that just sounds like comments with extra steps. What is the meaning of plain comments in these teams?


So, what do you mean by "addressed"? Because what you are describing is not a third state, but passive aggressive block.

You want to block me, but also want it to look like my decision instead of yours.


> So, what do you mean by "addressed"?

Doing "something", that solves the issue that is described in the comment?

That can range from actually changing the code, putting a comment in, over updating the documentation, moving the code somewhere else, to merging the PR unchanged and opening a new PR, to not doing anything.

> You want to block me, but also want it to look like my decision instead of yours.

Because it IS your decision. What the comment means for the effect on your PR depends on how you address it. "changing the code" -> "new code review"; "putting a comment in", "updating the documentation" -> don't need approval, but expect questions; "not doing anything" -> get your approval from someone else, you won't get it from me, but if someone else is fine with it, maybe I am wrong.

I DO NOT want to block you, that's why I want to be able to only leave a comment.

Examples:

"This conflicts with POSIX and SUSv4 ..." -> (but the old code also did) "document that diversion and leave a point in todo.txt"

"This is highly surprising for users" -> "add a warning or a confirmation"

"This breaks the workflow for X people (including me), but this was undocumented behaviour" -> "Consider whether the feature is worth it and we care about this"

"This conflicts with my understanding of RFC1234 in combination with 'random blog post from The Old New Thing' and documentation in 'different vendor's codebase'" -> "Yes, I know, I did it on purpose, because of footnote in RFC5678 and random comment on HN proving why Raymond Chen is wrong; will document that".

The answer to all these comments could also be "Oops, didn't thought of that, now I need to change the code / throw everything away.", but this entirely depends on your state of mind, which I don't have introspection to. If I would start researching that problem in depth, then I could also do the PR myself, that is work, that you already did. I only want to make sure, that you are aware. If you are, consider it approved, if you are not, consider it blocked.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: