Repository navigation
Remove targetDesc helper in assign.c in favor of splitting cases for i18n - #6489
Conversation
MichaelChirico
added
the
translation
|
Generated via commit 9266548 Download link for the artifact containing the test results: ↓ atime-results.zip Time taken to finish the standard R installation steps: 12 minutes and 6 seconds Time taken to run |
tdhock
left a comment
tdhock
left a comment
There was a problem hiding this comment.
looks fine. i wonder if it is worth adding a comment about the history with targetDesc? (ok if you dont think it is necessary)
In the code somewhere? (as a comment on the line where it used to be declared?) or here in the PR? |
|
I don't think a code comment is needed (it was not a particularly important part of the code, no "historic" relevance, and git is here for future archeologists), and we can edit the PR if needed later; submitting now. |
2 participants
Gave up on the grammatical case-switching approach suggested here:
#6482 (comment)
In the end dropping
targetDesc()altogether means about a dozen extra lines, I think the added clarity is worth it. As seen here, it also unconstrains us from shoehorning both branches into the same format, potentially adding clarity to the message.