It would be nice if the error messages generated would suggest replacement functions that they deem appropriate. I see that I'm not supposed to use gmtime, localtime, ctime, ctime_r, asctime, and asctime_r; but what do they think I should use?
> The ctime_r() and asctime_r() functions are reentrant, but have
no check that the buffer we pass in is long enough (the manpage says it
"should have room for at least 26 bytes"). Since this is such an
easy-to-get-wrong interface, and since we have the much safer strftime()
as well as its more convenient strbuf_addftime() wrapper, let's ban both
of those.
> The traditional gmtime(), localtime(), ctime(), and asctime() functions
return pointers to shared storage. This means they're not thread-safe,
and they also run the risk of somebody holding onto the result across
multiple calls (where each call invalidates the previous result). All callers should be using their reentrant counterparts.
I find it highly backwards that documentation on "what to use instead of X" is in the commit message disabling X. One _might_ do it and might remember to do it, but IMO it makes absolutely no sense for this not to be documented properly in code, as suggested by OP.
By that logic, a non-insignificant amount of (good) comments in code could be removed and people asked to "git blame the code and check out the commit that made it for the documentation". Of course this could be done, but it sounds ridiculous even typing it out.
I disagree. Commits messages exist for the very purpose of adding context to your code base. If you added <complex_function> for something that needs context, sure MAYBE add a comment, but I really pray that I'm going to find a few paragraphs disambiguating the problem within a git commit. If I'm _really_ lucky, maybe I find a PR number or Jira ticket reference as well.
If you're truly clueless as to what could be substituted for these commands, then you don't understand why they're banned. So our first step? Figure out why they're banned. And how would we sanely approach this? Probably by checking the commit message for _why that code is there in the first place_. That's a very safe, sane, and not-at-all backwards assumption. After you understand why it's there, a quick google search might help out if the commit message didn't already include information on alternatives.
Lastly, yeah, I totally agree a large amount of GOOD comments should be relegated to the git commits if all they're doing is adding additional context around a complex piece of logic. Comments do not exist to edifying a code base in any way other than context. They're too easy to let become stale, whereas a git commit will always reference exactly the code you're blaming.
So, I have to really disagree that it's ridiculous or in any way absurd. In fact, I think a lot of code suffers from NOT using git as a way to extend context around a code base. It's SUPER easy with most development environments to select a block of text and blame it. It's so easy that it's almost always my go-to to increase my context of what's been happening around a particular part of the code base.
What if that code is refactored, moved around and changed so many times that it's nearly impossible to find the "documentation" for the line you're interested in. I mean sure you could spend a few hours going though commit messages, but wouldn't it be nice if there was a simple comment next to the code that gives the info right away?
Also commits shouldn't be changed so if you want to improve the doc and provide more details, well you can't.
Granted these concerns are probably less likely to apply to this particular file.
Perhaps they just felt that anyone contributing to git would already know why not to use those/what to use instead (but then there would be no need to ban them).
> By that logic, a non-insignificant amount of (good) comments in code could be removed and people asked to "git blame the code and check out the commit that made it for the documentation". Of course this could be done, but it sounds ridiculous even typing it out.
Yes, exactly. You want to understand how a codebase changed and evolved over time? Git is your friend. If you want the facts of the code today? The source code is your friend. That's why the way Linux and Gits Git repository method of storing history makes sense. See also https://news.ycombinator.com/item?id=26348965
Try navigating the Git codebase with a git-blame sidebar (probably VS Code has that somewhere) so you can see the history of the source files. If you wonder why something is what it is, you can checkout the commit that last modified it. Or go even further backwards and figure out in the context it was first added. If you truly want to understand a change, a git repository with well written git messages is a pleasure to understand and dig into.
> Yes, exactly. You want to understand how a codebase changed and evolved over time? Git is your friend. If you want the facts of the code today? The source code is your friend.
100% agree. Though I don't mind if comments also leave historical information about the code. Can't be too much -- there is a delicate balance.
Do note, however, that you said it yourself: If you want the facts of the code today, go to the source code. In my opinion, the "facts of the code of git" are that functions X,Y,Z are "banned", but the code does not tell me why, or what to use instead. It just bans them. I would expect to see something in the code, not (just) in a git commit. It's also not that I can't google these functions (a couple of minutes will answer these questions), or that I should be experienced enough to know why they're evil, it's that it's IMO a reasonable, developer-friendly and good thing to do.
Why make it harder, and why make it impossible to update if there are other suggested alternatives that are available since whenever the commit was made?
Because there is no way for a commit message to become outdated or detached from what it talks about, both of which are very much issues with comments.
> why make it impossible to update if there are other suggested alternatives that are available since whenever the commit was made?
Ok, so maybe rather than have this file we should run “git log | grep BANNED” and build a list of functions from that? Or maybe we could change all error messages to be “go look at the commit history to work out why this happened”.
No? Maybe putting context in source files (or better yet, an error message!) rather than in a side channel like the commit message has value when it comes to understanding and updating, and it won’t be lost under the weight of future commits.
Your source code should describe what the program should do today. It should not contain all historical artifacts about your source code, as it'll grow to big and unmanageable then. Instead, use Git to store temporal information, data that is about change and reasoning behind it. Git is basically a timeline, instead of hard facts of today.
That's why it makes sense to describe the background and reasoning behind a change in a Git commit, instead of inside your source files as comments.
Totally agree, which is why nobody is suggesting adding the background and reasoning behind the change to the source file as a comment.
They are suggesting adding a more informative error, which may include a subset of that background and reasoning. An error message that points you to the functions you should use instead is infinitely more informative than one that says “this is banned. Bye.”
Code is evergreen, whereas a git commit represents a change at a single point in time. It will always be limited by the knowledge the author had available to them.
The commit message from 2020 with suggested alternatives might very well go stale. Does the author go and force a noop commit so they can document new best practice in a new commit message?
> Because there is no way for a commit message to become outdated or detached from what it talks about, both of which are very much issues with comments.
What if they think of another reason why one of the same functions should be disabled?
The UX of using this list is not by manually searching through the list and seeing the reason behind them. You include the file together with the rest of your sources and now you get compilation errors if you try to use them. Can't think of a better UX for banned functions.
Discovering why the thing is banned you only have to do once, if you care. If you're just modifying something quickly and minor in Git, you might not even care why.
Because that is effort every person who uses the file has to do over and over again, whereas maintaining the file is effort that has to be done once by one person.
Someone here commented to use git blame to find the commit that banned the functions and read the commits. These people making the suggestions.. must hate other people and their time. Also, what if someone.. for example runs a code formatter on the file, making git blame useless? Is it really so difficult to make a manual or explain properly in the comments about what replacements to use?
It sounds like you want a manual. Personal preference I guess. The maintainers seem to have decided to keep it in the history. It's not like this was ever meant for anything other than git itself.
Commit messages like that are common in the Linux kernel project, which is where git came from (though this particular commit message is a bit on the longer side).
It makes more sense if you think of it as an email message justifying why the project maintainer should accept that change, because that's what they were before git even existed. Still today, unless you're one of the Linux kernel subsystem maintainers, you have to convert your changes to emails with git-format-patch/git-send-email and send them to the right mailing list. Even the Linux kernel subsystem maintainers keep writing commits in that style out of habit (and because Linus will rant at them if they don't).
