Johan Herland wrote: > This initial implementation of 'git notes merge' only handles the trivial > merge cases (i.e. where the merge is either a no-op, or a fast-forward). Mm, sounds like a nice thing to have anyway. Now to digress: some generalities about test scripts. This will probably be very tedious. I'm writing it as groundwork before reviewing your test (since I don't feel on solid ground about this topic at all). More focused thoughts on other aspects of the patch will follow in a separate message. First: See http://thread.gmane.org/gmane.comp.version-control.git/155596/focus=155673 If you don't like what you see, no need to read the rest of this message. :) Motivation: Not being the best of shell script readers (or writers, for that matter), I need all the help from stylistic cues I can get. Old test scripts have a consistent style generally, but newer ones are starting to diverge from one another. A rough test format: #!/bin/sh # # Copyright (c) 2010 ... # test_description='... . ./test-lib.sh . ... constant data function definitions setup } test } test } repeat as necessary test } test_done Test description: test_description = 'One-line description Some words or diagrams describing the invariants (e.g., history) maintained between tests' Constant data: Simple commands to produce test vectors, expected output, and other variables and files that it might be convenient to have unchanged available throughout the test script. Because not guarded by a test assertion, this section would not include any git commands, nor any commands that might fail or write to the terminal. So: this section might use "cat <<\", "mkdir -p", "cp", "cp -R", "mv", "printf", "foo=", "unset", and "export", but not much else. Function definitions: Tests can define functions inside a test_expect_success block, too, but the generally useful functions would go up front with the constant data. Setup: test_expect_success '(setup | set up) ...' ' Commands to set up for the next batch of tests. Can rely on previous setup and can do just about anything. ' Tests: test_expect_success|failure '... some claim...' ' Commands to test that claim. Could do all sorts of things, as long as they do not disturb the invariants established by the constant_data and setup sections. ' What is not present: A test in this style would not include any commands except for test_done outside a test assertion after the first test_expect_success. This is meant to be a sort of compromise between what t/README says ("Put all code inside test_expect_success and other assertions") and what people actually do. I have no problem with people doing something else --- in fact, some tests would not make any sense in this style. In general, if a person is writing maintainable tests that do not slow down "make test" by much, it does not make much sense to put new obstacles in her way. What I expect is that new tests would be easier to read and extend in this style, since to get started, all a person has to pay attention to are the setup commands. So a potential collaborator could say "if you use this structure, it will make my life easier". Thoughts? Would it make sense to eventually put something like this in t/CodingStyle or nearby? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html