-
-
Notifications
You must be signed in to change notification settings - Fork 646
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cider-browse-ns.el (cider-browse-ns--doc-at-point): Fixup #1164
Conversation
Also, I see one instance of |
That would be a mistake. IIRC Could you update the commit message so it doesn't mention the file? Something like "Fix a malformed call-site" would be adequate. |
Btw, as this fixes a user-facing bug, it probably warrants a changelog entry as well. You can fix the |
* cider-browse-ns.el (cider-browse-ns--doc-at-point): Fixup. * CHANGELOG.md: Update.
* cider-browse-ns.el (cider-browse-ns--find-at-point): Update.
OK, updated. Btw, what's up with not mentioning the file in the commit message? I think it's a good practice. |
I'm sure @bbatsov has an opinion on this too but the biggest problem I had with the previous commit message was that the first line (the summay which appears in |
That's kind of the point. The commit message for such a minor fixup must look benign so that I can safely skip it when browsing the log. On the other hand, "Fix a malformed call-site" is ambiguous and sounds important enough to be tripping through it each time I'm looking for some change. Of course, that's just my opinion. I hope that you see that there's at least some logic to it:) |
cider-browse-ns.el (cider-browse-ns--doc-at-point): Fixup
Well, that's always subjective. :-) I'm a big believer in meaningful commit messages and clear commit titles. I don't like file mentions in commit messages because file names change and they don't add a ton of value - I'm generally interested in the problem and solution; the location is just details (and is often implied by the nature of the problem). |
previously.