Handle a quosure properly in strip_dots() and is_calculated() #3597
Conversation
|
Oh, maybe one minor issue: Can you amend the commit message so it states that the commit closes #3552? |
yutannihilation
force-pushed
the
fix/issue-3552-rlang-warnings
branch
from
1e69448 to
46478b5
Compare
|
Sure, I tweaked the commit message. But, I'm not familiar with this manner. Why should it be written in addition to the description of the pull request? |
|
I thought that github was automatically closing issues only based on commit messages, but now I see this may not be true. In any case, I think it makes sense to put it into the commit message, because that is part of the actual git log. If at some point in the future you stumble across this commit and you wonder why it was made then you can see directly from the commit message that it was related to a specific issue. If it's only mentioned in the PR, then you'd have to first dig out the PR that contributed the commit and then read through the discussion of that PR to find the same info. |
|
Thanks, makes sense. I tend to rewrite (or remove) the individual commit message when I "Squash and merge" on GitHub, but I'll keep in mind to include the original issue number in the message from now on! |
|
General advice is really just to put it in the PR description as that will close it. Any commit messages will potentially be removed during squashing anyway so it doesn’t make sense to have it there |
|
Yeah, ultimately, commit messages are not what we can rely on, but a useful commit message is still useful (e.g. Linux's commit messages are really cool). So, I just decided to do my best to write them a bit more informative. It's my personal decision and I don't think we can enforce it as a rule here, though. |
|
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
Fix #3552
Currently,
strip_dots()andis_calculated()treat a quosure as a call, and it surprisingly works for now, but it's fragile. A quosure should be treated as a quosure...