| blob | 13f9982bfa6ed00112a988f8b57bb6d30741cd18 |
6 <head>
9 </head>
12 <body>
15 <p>As header says, this chapter will be about commits, I already suppose that <b>you've set already your name and email correctly</b> (as shown in many tutorials and docs) using git-config, so I won't discuss here this thing. I won't teach you here the VIm editor (or any other $EDITOR you're using for commits), this chapter is just for additional information to people, who already make commits. I wrote this article sooner on TrinityCore forums, so it's just here as third chapter with some modifications.</p>
16 <p>note: This article is for everyone who will ever make any commits in Git, not just main devs, it's based on the basic knowledge that floats between Git users for a pretty long time, it's just here "again", so everyone will know about it (at least I hope so!).</p>
17 <hr />
18 <p>Hello everyone in the audience, today I'll talk about commits in Git. The talk should be taken as some "set of advices" which you don't have to follow, but everyone else (outside this project && using Git) does it in fact.</p>
20 <br />
23 <p>The first rule is basically to follow "one commit per feature / fix". That means making more smaller commits instead of comitting 5 things at once, so when something turns out as a bad idea later, you can easily use git-revert to revert it.</p>
24 <p>In SVN, that would mean 10000 commits instead of 3000 for exaple, however Git is constructed for large sets of small commits anyway, so it's a good idea even from the technical view.</p>
25 <br />
28 <p>The commit header is in fact the first line in commit message. It should contain some "title" for what the commit does, what it's actually about. Thanks to the first rule, it should be easy to make such title as the commit changes only one thing (fix/feature/part of the feature).</p>
30 <p>What is more important - the commit header is used by MANY git-related tools for it's mentioned purpose. Console gui tool <code>tig</code> uses it in commit list, git-format-patch uses it in email subject (IIRC), git-shortlog uses it for statistics and so on ..</p>
31 <p><u>The first line should be followed by an empty line and should not exceed 50 (at most 80) characters.</u></p>
32 <br />
35 <p>Git has the ability to inform you about trailing whitespaces (= whitespaces at the end of line, before newline, that are in fact useless), a space char followed by tab as indent (where only tab should be) and possibly other whitespace things that aren't enabled by default (see git-config's manpage). This is important to keep the code clean as much as possible.</p>
37 <p>LF/CRLF/CR problem is another issue. Linux systems use LF as a standard newline char, Microsoft Windows use CRLF (a carriage return followed by LF) and I think that Mac use just CR for that purpose (not sure about that).<br />
38 In fact it's a bad thing to have all three (or two) types of line endings in one repository even though the clients can convert them (client-side). LF is widely used as a standard in many repositories, so I recommend using is as well. Git has "autocrlf" option that can convert everything to LF or to CRLF, which is especially useful for Windows Git users (it's enabled in msysgit by default).</p>
40 <p>So triple check your code if it doesn't contain any whitespace errors (git-diff displays them as red blank fields when color is enabled) and if your autocrlf is enabled on Windows (or if you've used some external tool to convert everything to LF).</p>
41 <br />
46 <p>A very useful thingy when applying (git) patches. It does care about trailing whitespaces, space characters followed by tab as indent and possibly other whitespace things (again, it's in git-config's manpage). Use it! It will make your life easier!</p>
49 <p>You can actually use "less" pager to view CRs in "less" pager, it will display them as <code>^M</code>.<br />
53 <p>This will display any default whitespace errors and CRLFs (printed using invisible char in shell, use previous tip to catch them into "less") between two commits. Common use is just <code>git-diff HEAD^ --check</code> to check "errors" between previous and current commit.</p>
54 <hr />
55 <br />
57 <p>That covers most of the commit "mistakes" used in this project and things I've explained here <b>should be applied to not only git commits</b>. I hope you'll put to use what you've learned today (if you like it) and with that, I can now finish this talk.</p>
62 <hr />
63 <br />
66 Okay, so let me get straight more things which I thought are taken care of, obviously, they were not. I'll write some more examples of stuff you want to watch for IF you want to have clean commits. I'm talking theoretically all the time so for that reason, an example will also be included.</p>
69 <p>The first thing before (or after - you can --amend) you make a commit is to check the diff if there isn't any empty double newline near EOF, deleting and inserting a whole file because of CRLFs, reread your commit message if there aren't any "funny" typos and check the diff again.<br />
70 It's an important thing. In git, you can do this, because (as you already know) you make commits locally and thus can check all your commits if they fit and look nice before pushing them to public repo.</p>
73 <br />
77 I've saw it in many project with majority of MS Windows users, though I really don't know why, can't believe that all those users are still using MS Notepad.</p>
79 <pre>
80 [user@machine Updates]$ cat r594_mangos.sql
81 UPDATE `creature_template` SET `ScriptName`='mob_anubisath_sentinel' WHERE `entry`=15264;[user@machine Updates]$
82 </pre>
83 <p>As you can see, there's this <code>....WHERE `entry`=15264;[user@machine Updates]$ </code> thing, which is actually a newline error. If the file had contained a newline before EOF, the shell would print the prompt normally:</p>
84 <pre>
85 [user@machine Updates]$ cat r636_mangos.sql
86 UPDATE `creature_template` SET `ScriptName`='boss_laj' WHERE `entry`=17980;
87 [user@machine Updates]$
88 </pre>
89 <p>This is not only shell-related thing, many editors inserts that newline automatically, like VI(m), nano, ..., many other (even Windows) GUI editors, ..., however Microsoft Notepad for example doesn't.</p>
92 <pre>
93 [user@machine Updates]$ cat r121.sql
94 UPDATE `creature_template` SET `ScriptName` = 'mobs_spectral_ghostly_citizen' WHERE `entry` IN (10384, 10385);
96 [user@machine Updates]$
97 </pre>
98 <p>So please, watch this out, this thing (thanks to RULE 0) should not be in public repo as well.</p>
99 <br />
102 <p>Well I've promised it in the beginning, it's a typical commit taken from the Linux kernel changelog from the usb branch, please notice the commit header, line length and those Sign-offs (which are pretty standard as well, see manpage of git-commit for "-s" option):</p>
103 <pre>
104 commit 83a798207361cc26385187b2e71efa2b5d75de7f
108 USB: fix hcd interrupt disabling
110 Commit de85422b94ddb23c021126815ea49414047c13dc, 'USB: fix interrupt
111 disabling for HCDs with shared interrupt handlers' changed usb_add_hcd()
112 to strip IRQF_DISABLED from irqflags prior to calling request_irq()
113 with the justification that such a removal was necessary for shared
114 interrupts to work properly. Unfortunately, the change in that commit
115 unconditionally removes the IRQF_DISABLED flag, causing problems on
116 platforms that don't use a shared interrupt but require IRQF_DISABLED.
117 This change adds a check for IRQF_SHARED prior to removing the
118 IRQF_DISABLED flag.
120 Fixes the PS3 system startup hang reported with recent Fedora and
121 OpenSUSE kernels.
123 Note that this problem is hidden when CONFIG_LOCKDEP=y (ps3_defconfig),
124 as local_irq_enable_in_hardirq() is defined as a null statement for
125 that config.
132 </pre>
133 <br />
139 </body>
142 </html>