doc(proposal): add concerns to expectFailure label and/or matcher p…#18
doc(proposal): add concerns to expectFailure label and/or matcher p…#18vassudanagunta wants to merge 1 commit intonodejs:mainfrom
expectFailure label and/or matcher p…#18Conversation
There was a problem hiding this comment.
Thanks for the explanation.
I think this creates undue maintenance burden. If the user does something like what's outlined here, it will fail. This API isn't that complicated, and I can't think of any precedent in core where we do this much coddling, so IMO it's a case of "RTFM".
Please raise these sorts of things during the initial proposal; there was no revelation that revealed this concern. Proposing a change at the 11th hour creates exactly the churn the proposal process avoids 😕
In all fairness, I contributed a large amount of effort and time to the evolution of this proposal and the PR, all as a bystander, not as an official reviewer. I don't have the standing of a team member, so hesitated to make even more comments. Perhaps I should not be so shy. And very often "revelation" only happens when something is coded, when the implications of the design choices become apparent, especially as manifested by test cases. While I'm not in that camp, that's why some people push TDD so hard. In fact, as the "revelations" occurred, for me at least, I asked that test cases be added to make the design implications explicit, so that the test_runner team could take a second look before committing to such as significant change to the API. Better be safe than sorry. |
|
Indeed you did contribute a large amount of time and effort! My comment wasn't intended to scold you, if that's how it was received (and sorry if it did); it was merely to request, indeed, don't be shy, and to highlight the cost of delay. A proposal is exactly the time to throw out crazy ideas, edge-cases to potentially consider, etc 🙂 It also wasn't to dissuade you continuing. If you recall, I asked the implementor to hold tight a couple times (eg nodejs/node#61563 (comment)) during the proposal so we could get all the things ironed out and avoid churn, annnnd why I paused landing the PR to give you time to respond when I realised your unsettled concern.
My perception was that we did not treat you any differently. You technically don't get a vote, but I thought you had the space to speak your mind, and to my knowledge all your concerns were discussed and almost all resolved in a way that you were happy with? Churn is a very very real problem—quite possibly the biggest reason that we lose implementors. I could see the implementor was getting churn fatigue. Regarding my block on this PR: it mostly wasn't about the 11th hour¹. It was my vote that I believe the proposal is something we don't need now. But as I mentioned on the implementation PR: I'm open to being wrong, and if I am, I'm happy to revisit. Also note that it's "changes requested", not "drop it". I said I think it's not needed, not that it's inherently a bad idea. ¹ I blocked instead of just commented because I wanted to prevent idle discussion on this to stymie the implementation at the finish-line. Implementing this proposal wouldn't be a breaking change, so it doesn't need to block the current. |
Per nodejs/node#61563 (comment)