Conversation
CT Test Results 1 files 11 suites 3m 49s ⏱️ Results for commit bef66e1. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts
// Erlang/OTP Github Action Bot |
bjorng
left a comment
There was a problem hiding this comment.
Thanks for your pull request. This looks good except for one change. Please change it back or give me a good motivation why your version is better than the original.
| @@ -305,8 +305,8 @@ linter. It extracts arities from all defined functions. All elements in the | |||
| list `DefinedFuns` are two-tuples, containing name and arity for functions. | |||
| If any of them differs from this pattern, it means that something has added | |||
| an invalid item into the list of defined functions. It is better for the linter | |||
| to crash in the comprehension than skipping the invalid item and continue | |||
| running. Using a strict generator here is correct, because the linter should | |||
| to crash in the comprehension than skipping the invalid item and continuing to | |||
There was a problem hiding this comment.
Can you motivate this change? My own Sprachgefühl likes the original wording better. Also, two LLMs I asked changed your version back to the original, and one LLM explained that since "crash" and "skip" are infinitives, "continue" should also be in the infinitive form.
There was a problem hiding this comment.
Well... the original wording was not entirely correct, but my change made it worse XP The correct wording (also checked by an LLM) would be "It is better for the linter to crash in the comprehension than to skip the invalid item and continue running."
Will change 👍
Co-authored-by: Maria Scott <maria-12648430@hnc-agency.org>
fe832b8 to
bef66e1
Compare
No description provided.