bug-diffutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug-diffutils] [PATCH] doc: add contributor guidelines: HACKING


From: Jim Meyering
Subject: [bug-diffutils] [PATCH] doc: add contributor guidelines: HACKING
Date: Thu, 16 Dec 2010 15:22:36 +0100

FYI, I've just pushed this.
It probably has minor errors, since I barely skimmed it.
Corrections welcome.

>From 5c65882affa003bab8b21c6f1cdfa038d51b5ca6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Thu, 16 Dec 2010 15:20:24 +0100
Subject: [PATCH] doc: add contributor guidelines: HACKING

* HACKING: New file, copied 99% from the one in grep's repository.
---
 HACKING |  591 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 591 insertions(+), 0 deletions(-)
 create mode 100644 HACKING

diff --git a/HACKING b/HACKING
new file mode 100644
index 0000000..bcd555b
--- /dev/null
+++ b/HACKING
@@ -0,0 +1,591 @@
+Diffutils 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.sv.gnu.org/diffutils
+
+That downloads the entire repository, including revision control history
+dating back to 1991.  The repository (the part you download, and which
+resides in diffutils/.git) currently weighs in at about 9MB.  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 diffutils/ 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 = address@hidden
+
+
+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 diffutils/ 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 diffutils like this:
+
+  git clone git://git.sv.gnu.org/diffutils
+
+Now, cd into the diffutils/ 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 diffutils, 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.
+
+
+
+diffutils-specific:
+
+No more ChangeLog files
+=======================
+Do not modify any of the ChangeLog files in diffutils.  Starting in
+2010, 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 diffutils working directory name matches,
+this code enables the right mode:
+
+  ;; In diffutils, indent with spaces everywhere (not TABs).
+  ;; Exceptions: Makefile and ChangeLog modes.
+  (add-hook 'find-file-hook '(lambda ()
+    (if (and buffer-file-name
+             (string-match "/diffutils\\>" (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 diffutils'
+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 diffutils -q -d lib -b lib -o lib.lcov -c
+  lcov -t diffutils -q -d src -b src -o src.lcov -c
+  # generate HTML from the output
+  genhtml -p `pwd` -t diffutils -q --output-directory lcov-html *.lcov
+
+Then just open the index.html file (in the generated lcov-html directory)
+in your favorite web browser.
+
+========================================================================
+Copyright (C) 2009-2010 Free Software Foundation, Inc.
+
+Permission is granted to copy, distribute and/or modify this document
+under the terms of the GNU Free Documentation License, Version 1.3 or
+any later version published by the Free Software Foundation; with no
+Invariant Sections, with no Front-Cover Texts, and with no Back-Cover
+Texts.  A copy of the license is included in the ``GNU Free
+Documentation License'' file as part of this distribution.
--
1.7.3.4



reply via email to

[Prev in Thread] Current Thread [Next in Thread]