diff options
author | Alexander Todorov <atodorov@redhat.com> | 2015-07-06 14:38:26 +0300 |
---|---|---|
committer | Brian C. Lane <bcl@redhat.com> | 2015-07-13 14:30:07 -0700 |
commit | 6e7e98b09dd7d0e826c5e1d62d0819513785fbd2 (patch) | |
tree | 50ed006260f9214813df4445f884e7768f396f16 /README-hacking | |
parent | 282e25e0384e8d1275ccacf904fdaf65f1d4a8af (diff) | |
download | parted-6e7e98b09dd7d0e826c5e1d62d0819513785fbd2.tar.gz |
merge HACKING and README-hacking
Diffstat (limited to 'README-hacking')
-rw-r--r-- | README-hacking | 582 |
1 files changed, 581 insertions, 1 deletions
diff --git a/README-hacking b/README-hacking index 97ce115..aeacabb 100644 --- a/README-hacking +++ b/README-hacking @@ -65,7 +65,587 @@ and the GIT master copy: should output no difference. -Enjoy! ++-*- Parted Contribution Guidelines -*- + + +Prerequisites +============= +You will need the "git" version control tools. +On Fedora-based systems, do "yum install git". +On Debian-based ones install the "git-core" package. +Then run "git --version". If that says it's older than +version 1.4.4, then you'd do well to get a newer version. +At worst, just download the latest stable release from +http://git.or.cz/ and build from source. + +For details on building the programs in this package, see +the file, README-hacking. + + +Use the latest upstream sources +=============================== +Base any changes you make on the latest upstream sources. +You can get a copy of the latest with this command: + + git clone git://git.debian.org/git/parted/parted.git + +That downloads the entire repository, including revision control history +dating back to 1991. The repository (the part you download, and which +resides in parted/.git) currently weighs in at about 2.6MB. So you +don't want to download it more often than necessary. Once downloaded, +you can get incremental updates by running one of these commands from +inside your new parted/ directory: + +If you have made *no* changes: + git pull + +If you *have* made changes and mistakenly committed them to "master", +do the following to put your changes on a private branch, "br", and +to restore master to its unmodified (relative-to-upstream) state: + git checkout -b br + git checkout master + git reset --hard origin + +Then "git pull" should work. + + +*Before* you commit changes +=========================== + +In this project, we much prefer patches that automatically record +authorship. That is important not just to give credit where due, but +also from a legal standpoint (see below). To create author-annotated +patches with git, you must first tell git who you are. That information +is best recorded in your ~/.gitconfig file. Edit that file, creating +it if needed, and put your name and email address in place of these +example values: + +[user] + name = Joe X. User + email = joe.user@example.com + + +Your first commit: the quick and dirty way +========================================== +First of all, realize that to "commit" a change in git is a purely +local operation. It affects only the local repository (the .git/ dir) +in your current parted/ hierarchy. + +To try this out, modify a file or two. If you create a new file, you'll +need to tell git about it with "git add new-file.c". Commit all changes +with "git commit -a". That prompts you for a log message, which should +include a one-line summary, a blank line, and ChangeLog-style entries +for all affected files. More on that below. + +Once your change is committed, you can create a proper patch that includes +a log message and authorship information as well as any permissions +changes. Use this command to save that single, most-recent change set: + + git format-patch --stdout -1 > DIFF + +The trouble with this approach is that you've just checked in a change +(remember, it's only local) on the "master" branch, and that's where new +changes would normally appear when you pull the latest from "upstream". +When you "pull" from a remote repository to get the latest, your local +changes on "master" may well induce conflicts. For this reason, you +may want to keep "master" free of any local changes, so that you can +use it to track unadulterated upstream sources. + +However, if your cloned directory is for a one-shot patch submission and +you're going to remove it right afterwards, then this approach is fine. +Otherwise, for a more sustainable (and more generally useful, IMHO) +process, read on about "topic" branches. + + +Make your changes on a private "topic" branch +============================================= +So you checked out parted like this: + + git clone git://git.debian.org/git/parted/parted.git + +Now, cd into the parted/ directory and run: + + git checkout -b my-topic + +That creates the my-topic branch and puts you on it. +To see which branch you're on, type "git branch". +Right after the clone, you were on "master" (aka the trunk). +To get back to the trunk, do this: + + git checkout master + +Note 1: + Be careful to run "git pull" only when on the "master" branch, + not when on a branch. With newer versions of git, you can't cause + trouble if you forget, so this is a good reason to ensure you're + using 1.5.3.1 or newer. + +Note 2: + It's best not to try to switch from one branch to another if + you have pending (uncommitted) changes. Sometimes it works, + sometimes the checkout will fail, telling you that your local + modifications conflict with changes required to switch branches. + However, in any case, you will *not* lose your uncommitted changes. + +Anyhow, get back onto your just-created branch: + + git checkout my-topic + +Now, modify some file and commit it: + + git commit some-file.c + +Personally, no matter what package I'm working on, I find it useful to +put the ChangeLog entries *only* in the commit log, initially, unless +I plan to commit/push right away. Otherwise, I tend to get unnecessary +merge conflicts with each rebase (see below). In parted, I've gone +a step further, and no longer maintain an explicit ChangeLog file in +version control. Instead, in a git working directory, you can view +ChangeLog information via "git log". However, each distribution tarball +does include a ChangeLog file that is automatically generated from the +git logs. + +So, you've committed a change. But it's only in your local repository, +and only on your "my-topic" branch. Let's say you wait a day, and +then see that someone else changed something and pushed it to the +public repository. Now, you want to update your trunk and "rebase" +your changes on the branch so that they are once again relative to the +tip of the trunk. Currently, your branch is attached to the trunk at +the next-to-last change set. + +First: update the trunk from the public repo: +[you've first made sure that "git diff" produces no output] + + git checkout master + git pull + +Now, return to your branch, and "rebase" relative to trunk (master): + + git checkout my-topic + git rebase master + +If there are no conflicts, this requires no more work from you. +However, let's say there was one in ChangeLog, since you didn't +follow my advice and modified it anyway. +git rebase will tell you there was a conflict and in which +file, and instruct you to resolve it and then resume with +"git rebase --continue" once that's done. + +So you resolve as usual, by editing ChangeLog (which has the +usual conflict markers), then type "git rebase --continue". +That will fail, with a diagnostic telling you to mark +the file as "conflict resolved" by doing this: + + git add ChangeLog + +Then, finally, you can proceed (possibly onto more conflict resolution, +if there are conflicts in other files): + + git rebase --continue + +Once it finishes, your changes on the branch are now relative to +the tip of the trunk. + +Now use git format-patch, as above. + + +Amending the most recent change on your private branch +====================================================== +Let's say you've just committed a change on your private +branch, and then realize that something about it is not right. +It's easy to adjust: + + edit your files # this can include running "git add NEW" or "git rm BAD" + git commit --amend -a + git format-patch --stdout -1 > your-branch.diff + +That replaces the most recent change-set with the revised one. + + + +Parted-specific: + +No more ChangeLog files +======================= +Do not modify any of the ChangeLog files in parted. Starting in +2008, the policy changed. Before, we would insert the exact same text +(or worse, sometimes slightly differing) into both the ChangeLog file +and the commit log. Now we put that information only in the commit log, +and generate the top-level ChangeLog file from logs at "make dist" time. +As such, there are strict requirements on the form of the commit log +messages. + + +Commit log requirements +======================= +Your commit log should always start with a one-line summary, the second +line should be blank, and the remaining lines are usually ChangeLog-style +entries for all affected files. However, it's fine -- even recommended -- +to write a few lines of prose describing the change, when the summary +and ChangeLog entries don't give enough of the big picture. Omit the +leading TABs that you're used to seeing in a "real" ChangeLog file, but +keep the maximum line length at 72 or smaller, so that the generated +ChangeLog lines, each with its leading TAB, will not exceed 80 columns. +As for the ChangeLog-style content, please follow these guidelines: + + http://www.gnu.org/software/guile/changelogs/guile-changelogs_3.html + +Try to make the summary line fit one of the following forms: + + program_name: change-description + prog1, prog2: change-description + doc: change-description + tests: change-description + build: change-description + maint: change-description + + +Curly braces: use judiciously +============================= +Omit the curly braces around an "if", "while", "for" etc. body only when +that body occupies a single line. In every other case we require the braces. +This ensures that it is trivially easy to identify a single-*statement* loop: +each has only one *line* in its body. + +Omitting braces with a single-line body is fine: + + while (expr) + single_line_stmt (); + +However, the moment your loop/if/else body extends onto a second line, +for whatever reason (even if it's just an added comment), then you should +add braces. Otherwise, it would be too easy to insert a statement just +before that comment (without adding braces), thinking it is already a +multi-statement loop: + + while (true) + /* comment... */ // BAD: multi-line body without braces + single_line_stmt (); + +Do this instead: + + while (true) + { /* Always put braces around a multi-line body. */ + /* explanation... */ + single_line_stmt (); + } + +There is one exception: when the second body line is not at the same +indentation level as the first body line. + + if (expr) + error (0, 0, _("a diagnostic that would make this line" + " extend past the 80-column limit")); + +It is safe to omit the braces in the code above, since the +further-indented second body line makes it obvious that this is still +a single-statement body. + +To reiterate, don't do this: + + if (expr) + while (expr_2) // BAD: multi-line body without braces + { + ... + } + +Do this, instead: + + if (expr) + { + while (expr_2) + { + ... + } + } + +However, there is one exception in the other direction, when even a +one-line block should have braces. That occurs when that one-line, +brace-less block is an "else" block, and the corresponding "then" block +*does* use braces. In that case, either put braces around the "else" +block, or negate the "if"-condition and swap the bodies, putting the +one-line block first and making the longer, multi-line block be the +"else" block. + + if (expr) + { + ... + ... + } + else + x = y; // BAD: braceless "else" with braced "then" + +This is preferred, especially when the multi-line body is more than a +few lines long, because it is easier to read and grasp the semantics of +an if-then-else block when the simpler block occurs first, rather than +after the more involved block: + + if (!expr) + x = y; /* more readable */ + else + { + ... + ... + } + +If you'd rather not negate the condition, then add braces: + + if (expr) + { + ... + ... + } + else + { + x = y; + } + + +Use SPACE-only indentation in all[*] files +========================================== +We use space-only indentation in nearly all files. +If you use Emacs and your parted working directory name matches, +this code enables the right mode: + + ;; In parted, indent with spaces everywhere (not TABs). + ;; Exceptions: Makefile and ChangeLog modes. + (add-hook 'find-file-hook '(lambda () + (if (and buffer-file-name + (string-match "/parted\\>" (buffer-file-name)) + (not (string-equal mode-name "Change Log")) + (not (string-equal mode-name "Makefile"))) + (setq indent-tabs-mode nil)))) + +[*] Makefile and ChangeLog files are exempt, of course. + +[FIXME: suggest vim syntax to do same thing, if it can be done safely. + Most distros now "set nomodeline" by default for a good reason. ] + + +Send patches to the address listed in --help output +=================================================== +Please follow the guidelines in the "Sending your patches." section of +git's own SubmittingPatches: + + http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/SubmittingPatches + + +Add documentation +================= +If you add a feature or change some user-visible aspect of a program, +document it. If you add an option, document it both in --help output +(i.e., in the usage function that generates the --help output) and in +doc/*.texi. The man pages are generated from --help output, so +you shouldn't need to change anything under man/. User-visible changes +are usually documented in NEWS, too. + +When writing prose (documentation, comments, log entries), use an +active voice, not a passive one. I.e., say "print the frobnozzle", +not "the frobnozzle will be printed". + +Please add comments per the GNU Coding Standard: + http://www.gnu.org/prep/standards/html_node/Comments.html + + +Minor syntactic preferences +=========================== +[I hesitate to write this one down, because it appears to be an + acquired taste, at least for native-English speakers. It seems odd + (if not truly backwards) to nearly anyone who doesn't have a strong + mathematics background and perhaps a streak of something odd in their + character ;-) ] +In writing arithmetic comparisons, use "<" and "<=" rather than +">" and ">=". For some justification, read this: + http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=4126 + +const placement: +Write "Type const *var", not "const Type *var". +FIXME: dig up justification + + +Be nice to translators +====================== +Don't change translatable strings if you can avoid it. +If you must rearrange individual lines (e.g., in multi-line --help +strings), extract and create new strings, rather than extracting +and moving into existing blocks. This avoids making unnecessary +work for translators. + + +Add tests +========== +Nearly every significant change must be accompanied by a test suite +addition that exercises it. If you fix a bug, add at least one test that +fails without the patch, but that succeeds once your patch is applied. +If you add a feature, add tests to exercise as much of the new code +as possible. Note to run tests/new-test in isolation you can do: + + (cd tests && make check TESTS=new-test VERBOSE=yes) + +There are many tests in the tests/ directories. Use one of the +init.sh-using scripts as a template. + +If writing tests is not your thing, don't worry too much about it, +but do provide scenarios, input/output pairs, or whatever, along with +examples of running the tool to demonstrate the new or changed feature, +and someone else will massage that into a test (writing portable tests +can be a challenge). + + +Copyright assignment +==================== +If your change is significant (i.e., if it adds more than ~10 lines), +then you'll have to have a copyright assignment on file with the FSF. +Since that involves first an email exchange between you and the FSF, +and then the exchange (FSF to you, then back) of an actual sheet of paper +with your signature on it, and finally, some administrative processing +in Boston, the process can take a few weeks. + +The forms to choose from are in gnulib's doc/Copyright/ directory. +If you want to assign a single change, you should use the file, +doc/Copyright/request-assign.changes: + + http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=doc/Copyright/request-assign.changes;hb=HEAD + +If you would like to assign past and future contributions to a project, +you'd use doc/Copyright/request-assign.future: + + http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=blob;f=doc/Copyright/request-assign.future;hb=HEAD + +You may make assignments for up to four projects at a time. + +In case you're wondering why we bother with all of this, read this: + + http://www.gnu.org/licenses/why-assign.html + + +Run "make syntax-check", or even "make distcheck" +================================================ +Making either of those targets runs many integrity and +project-specific policy-conformance tests. For example, the former +ensures that you add no trailing blanks and no uses of certain deprecated +functions. The latter performs all "syntax-check" tests, and also +ensures that the build completes with no warnings when using a certain +set of gcc -W... options. Don't even bother running "make distcheck" +unless you have a reasonably up to date installation including recent +versions of gcc and the linux kernel, and modern GNU tools. + + +Ensure that your changes are indented properly. +=============================================== +Format the code the way GNU indent does. +In a file with the "indent-tabs-mode: nil" directive at the end, +running "indent --no-tabs" should induce no change. +With other files, there will be some existing differences. +Try not to add any more. + + +Avoid trailing white space +========================== +You may notice that the only trailing blanks in parted's +version-controlled files are in a single directory: tests/pr, +which contains expected output from various invocations of pr. + +Do not add any more trailing blanks anywhere. While "make syntax-check" +will alert you if you slip up, it's better to nip any problem in the +bud, as you're typing. A good way to help you adapt to this rule is +to configure your editor to highlight any offending characters in the +files you edit. If you use Emacs, customize its font-lock mode (FIXME: +provide more detail) or try one of its whitespace packages. This appears +to be the one that will end up in emacs 23: + + http://www.emacswiki.org/emacs/WhiteSpace + +[that page says its version also works with emacs 21 and 22] +If you use vim, add this to ~/.vimrc: + + let c_space_errors=1 + highlight RedundantSpaces ctermbg=red guibg=red + match RedundantSpaces /\s\+$\| \+\ze\t/ + + +Git can help too, by stopping you from committing any change that would +add trailing blanks. The example pre-commit hook contains code to check +for trailing whitespace and spaces before tabs; enable it by moving it +to the right place and making sure it is executable: + + mv .git/hooks/pre-commit.sample .git/hooks/pre-commit + +With a repository created by git-1.5.6 or older, use this command: + + chmod +x .git/hooks/pre-commit + +To manually check for whitespace errors before committing, you can use + + git diff --check + +Git also has some settings to enable suitable internal whitespace checks. +See the manpage for git-apply for details. + + +------------------------------------------- + +Miscellaneous useful git commands +================================= + + * gitk: give a graphical view of the revision graph of the current branch + * gitk --all: same, but display all branches + * git log: to get most of the same info in text form + * git log -p: same as above, but with diffs + * git log -p SOME_FILE: same as above, but limit to SOME_FILE + * git log -p -2 SOME_FILE: same as above, but print only two deltas + * git log -p -1: print the most recently committed change set + * git format-patch --stdout -1 > FILE: output the most recently committed + change set, in a format suitable to be submitted and/or applied via + "git am FILE". + * git reset --soft HEAD^: Commit the delta required to restore + state to the revision just before HEAD (i.e., next-to-last). + * git rebase -i master: run this from on a branch, and it gives + you an interface with which you can reorder and modify arbitrary + change sets on that branch. + + * if you "misplace" a change set, i.e., via git reset --hard ..., so that + it's no longer reachable by any branch, you can use "git fsck" to find + its SHA1 and then tag it or cherry-pick it onto an existing branch. + For example, run this: + git fsck --lost-found HEAD && cd .git/lost-found/commit \ + && for i in *; do git show $i|grep SOME_IDENTIFYING_STRING \ + && echo $i; done + The "git fsck ..." command creates the .git/lost-found/... hierarchy + listing all unreachable objects. Then the for loop + print SHA1s for commits that match via log or patch. + For example, say that found 556fbb57216b119155cdda824c98dc579b8121c8, + you could run "git show 556fbb57216b119" to examine the change set, + or "git checkout -b found 556fbb5721" to give it a branch name. + Finally, you might run "git checkout master && git cherry-pick 556fbb5721" + to put that change on the tip of "master". + +------------------------------------------- + +Finding things to do +==================== +If you don't know where to start, check out the TODO file for projects +that look like they're at your skill-/interest-level. Another good +option is always to improve tests. You never know what you might +uncover when you improve test coverage, and even if you don't find +any bugs your contribution is sure to be appreciated. + +A good way to quickly assess current test coverage is to use "lcov" +to generate HTML coverage reports. Follow these steps: + + # configure with coverage information + ./configure CFLAGS="-g -fprofile-arcs -ftest-coverage" + make + # run whatever tests you want, i.e.: + make check + # run lcov + lcov -t parted -q -d libparted -b lib -o lib.lcov -c + lcov -t parted -q -d parted -b src -o src.lcov -c + # generate HTML from the output + genhtml -p `pwd` -t parted -q --output-directory lcov-html *.lcov + +Then just open the index.html file (in the generated lcov-html directory) +in your favorite web browser. ----- |