wiki:WeblocksDevelopment
close Warning: Can't synchronize with repository "(default)" (/project/cl-weblocks/svn does not appear to be a Subversion repository.). Look in the Trac log for more information.

Branches And Development Process

Weblocks development process occurs in github repository https://github.com/skypher/weblocks

Testing

If you introduce new features, please provide unit tests. If you fix an untested bug, test for that as well. We're happy to merge features we won't be using ourselves if it's made reasonably robust via testing; otherwise, we're likely to break it with other changes.

A corollary is don't break tests. Getting all the tests to succeed again after the big post-Darcs merge was hard enough; no one wants to let it drift out of control again, and we'd rather let features lie than break a bunch of tests again. If you break a test and think the test itself should change or be removed, think twice before doing so, because if the complexity of test changes outweigh the source-side benefits of a change, we won't merge it.

Upon introduction of features that change CSS classes, introduce new HTML containers or remove old ones you also need to make sure that the default CSS rules still apply. A rough way of checking this is running the demo applications. If the CSS needs to be changed, include it in your patch.

The demo applications must be checked to still work after your changes. If they don't then please include the appropriate changes to make them work again in your patch, too.

Submitting Patches

Developing and maintaining Weblocks is fun, but it takes a lot of time. We very much appreciate people submitting patches and we'd like to accept as many useful patches as possible, but unfortunately some submissions require us to carve out chunks of time that we do not have. To make the maintenance process easier on the core team and to increase the likelihood of getting your patch accepted, please follow the guidelines below:

  1. Add a documentation string for every function/class/slot/parameter you're adding. If you're modifying something, please be sure to also modify relevant documentation strings.
  2. Unit test every feature you're adding. If you feel something you've added isn't possible to unit test in a useful manner, let me know about it.
  3. Follow the overall code and documentation style of Weblocks codebase. I know it's not perfect, and one day I may write scripts to change it, but consistency is more valuable than tiny bits of styling perfection.
  4. Integrate your changes into Weblocks codebase.
    1. If you're adding new files, also add them to relevant ASDF files.
    2. Put files in the appropriate location (parts of weblocks under src, unit tests under test, scripts under scripts, publicly downloadable files under pub, etc.) If you're not sure where to put something, ask me.
    3. If you're adding new unit tests, make sure they don't affect the testing environment in a way that may break other tests.
    4. Make sure existing unit tests pass.

I just want to try contributing one or two small patches…

To submit the patches please publish your repository clone on http://github.com (or elsewhere) and initiate a pull request on the appropriate branch's github page. To publish on Github, fork Weblocks, clone the fork locally, and make all your changes in that clone. Initiate the pull request for the appropriate repository (see branch descriptions above). Detailed Github steps follow.

  1. Log in to github.com
  2. Go to the appropriate repository, normally https://github.com/skypher/weblocks
  3. Click the "fork" button.
  4. Install Git on a machine somewhere; I'll assume you're using the command line interface.
  5. Go to your fork in github; get the Clone URL
  6. Run: git clone [Clone URL]
  7. Make your edits for the patch you want to submit
  8. Run: git commit
  9. Run: git push
  10. Wait a bit, then check your forked repo on github for the changes
  11. Go to "Pull requests" tab on your clone page.
  12. Initiate pull request.
  13. Explain your changes in the text box
  14. Click send request
  15. Leave the branch in place until the devs say they are done with it

I want to develop a new feature without continually merging dev.

That's fine, but before we will accept, you need to do a trial merge. I (S11) keep a pristine dev repository locally for just this purpose. Pull and merge the latest dev in, fix conflicts, and test it all again to make sure it still works. If the conflicts aren't trivial, you may want to push the merge anyway, just so we don't make a mistake when fixing them. If a test breaks after merging dev because dev "tests harder", you can put the fix in before the merge instead of pushing the merge with the fix on top if you like, because our intention in such cases is usually to say "Weblocks should always behave like this".

A good reason for conflicts in trial merges is that we've already pushed something similar to dev. So if your change is small, make sure someone hasn't already done the work you're doing in dev itself.

You might consider using Clozure for speed when doing trial merges if you are using SBCL or CMUCL. Clozure has a much faster compiler and fasl-loader, which can be helpful if you are frequently clearing out stale fasls and rebuilding from scratch.

I want to track dev while working on my changes.

The best thing to do is regularly pull from dev, merge, test, and keep working on the branch.

But now you've heard that there's a way to have your topic branch track dev without having the merge revisions appear in your log. It's called rebase, and its use is rife with problems.

An ASCII-art representation of how rebase works can be seen in the git-rebase(1) man page. A more colorful one can be seen on S11's blog.

If you use it, you'll be seriously impeding or breaking:

  • Pushing your branch to Bitbucket. Pushing only creates new revs, and rebase implies "stripping" the old version of each rebased rev. But see Bitbucket meta #402 for the somewhat evil change request to fix this. A little analysis of Bitbucket's features will show why this is not a nice feature.
  • Sharing incomplete features, accepting feedback and hacks from others. Cross-merging makes sure everyone involved is working from the same base, and reliably records when different ideas were implemented in revs for the feature and when changes from multiple hackers were put together . Rebasing wipes out much of this information, and more seriously, causes the "cascading rebase" problem detailed in the same blog entry mentioned above.

So the existence of rebase encourages you to consider that "clean history", whatever that means, might be more important than the above things. But these things are really good. Being able to share a contribution to someone else's topic branch, without worrying that you might have to later manually analyze the whole topic history again to complete a cascade, is good. Letting other people provide ideas for a topic before fully investing in a complete, ready-for-dev topic branch is good. Is it really terribly important to not have to pass -M to hg log, or not to wait for Bitbucket to implement "don't show merge revs" and "don't show revs in fork parent"?

Even hackers who take the time to implement a "rebase" function think it's a bad idea pretty often. So take a minute to consider whether it is really a good idea for you. From the git-rebase(1) man page:

Rebasing (or any other form of rewriting) a branch that others have based work on is a bad idea: anyone downstream of it is forced to manually fix their history. This section explains how to do the fix from the downstream's point of view. The real fix, however, would be to avoid rebasing the upstream in the first place.

If you want to do it anyway, here you go.

$ ls
dev/  my-topic/
$ (cd dev && hg pull -u)        # update "pristine" local repo
pulling from http://bitbucket.org/S11001001/weblocks-dev/
searching for changes
adding changesets
adding manifests
adding file changes
added 2 changesets with 4 changes to 3 files
$ cd my-topic
$ hg pull ../dev                # pull in new dev changes
pulling from ../dev
searching for changes
adding changesets
adding manifests
adding file changes
added 2 changesets with 3 changes to 2 files (+1 heads)
(run 'hg heads' to see heads, 'hg merge' to merge)
$ hg heads
# Note the higher number (tip) is the just-pulled revs
changeset:   1176:56a0f397355b
tag:         tip
user:        Stephen Compall <scompall@nocandysw.com>
date:        Sun Jan 11 17:21:21 2009 -0600
summary:     Build date printing around metatilities:format-date instead, testing

# And here is the alternate head created by your topic changes
changeset:   1174:cd08a2361295
user:        Stephen Compall <scompall@nocandysw.com>
date:        Fri Jan 16 22:50:50 2009 -0600
summary:     my-topic: some other change I made
$ hg rebase -d tip      # do the rebase (only works when `hg id` is cd08a23)
merging weblocks.asd
merging weblocks-test.asd
saving bundle to /home/sirian/lisp/weblocks/my-topic/.hg/strip-backup/d72aa32aabad-temp
adding branch
adding changesets
adding manifests
adding file changes
added 4 changesets with 6 changes to 4 files    # 2 added + 2 in topic
rebase completed
$ hg heads
changeset:   1176:00618ea0af7e
tag:         tip
user:        Stephen Compall <scompall@nocandysw.com>
date:        Fri Jan 16 22:50:50 2009 -0600
summary:     my-topic: some other change I made

I have a bunch of unrelated changes in my fork, but you tend to cherry them.

This is a good reason to watch how we tend to adopt your changes. Different people have different approaches to hacking Weblocks, and this encourages us to think differently about how to pull in changes from each person.

You can help us, and yourself by adapting your workflow to how we pull in your changes.

If we pull in your stuff wholesale, you might as well just keep committing to your repo, tracking dev or stable as necessary. There's no benefit to the more complex systems if we don't cherry from you.

If we're cherrying alot from you, that makes hg out mostly useless for figuring out what divergent changes you still carry around. In that case, you should try one of the patch management systems for Mercurial. If you tend to make each change in a single revision, Mercurial Queues are what you want. If you tend to work on different topics in multiple revisions, sometimes overlapping, try patch branches. With either of these systems, watch us for what we pull in; that way, you can remove these patches from your repository when appropriate, and the remaining list of patches reflects what divergences you still carry.

You merged my changes, now what?

Observe our commentary. Often, a push of a merge of your changes will also include the merger's changes. You can consider this a commentary on your changes described in software. We rarely announce such things because merge-with-commentary is such a common thing in free software projects, so you should watch dev after your changes are merged in for their appearance. Just because they don't appear immediately doesn't mean there won't be commentary; a mainline developer other than the merger might have something to say.

If you disagree with a commentary so much that you want it changed back you have to tell us. Further changes of your own that don't include the commentary will simply be treated as ordinary conflicts and fixed as if the commentary's inclusion is assumed. This is because your disagreement isn't reflected in an actual Mercurial revision; it is just a divergence. If you disagree with a commentary, but only that it should be changed or enhanced, the appropriate thing is to pull in the commentary and make further changes on top of it. This way, the revision graph reflects the actual conversation behind the changes.

Now for the technical side: get the hgk extension, because it gives you useful revision graphing. Some Weblocks developers arrange commits so that commentaries appear as direct children of the revisions they comment upon. Sometimes. When they feel up to it. Here's an interesting case of a dual parallel commentary appearing in an unusually complex web of merges. Different connections are in different colors for contrast, and the colors of text labels indicate different authors:

http://common-lisp.net/project/cl-weblocks/images/template-block-comm-1.png

Ignore the web, focus on the fuschia line. No, not the one on the left that goes up into oblivion. Start at added template-block-mixin. Okay, ignore the little one that goes to the right, that's just another change from you. Focus on the other things it leads to:

http://common-lisp.net/project/cl-weblocks/images/template-block-comm-2.png

Remember that further commentaries may follow on a single line; this was just an instance where they were made separately.

Here's another, longer example brought on by the interesting graphing algorithm:

http://common-lisp.net/project/cl-weblocks/images/ffmsg-1.png

Hint: it's the big brown one on the right.

http://common-lisp.net/project/cl-weblocks/images/ffmsg-2.png

This is a backout, meaning that the commentary means to say that the change should be reverted entirely. If you have dev, find this revision and look at the merge that follows it for an explanation of why it was backed out.

Anyway, using strict DAGs for revision graphing means it's kind of a pain to use direct-child commentary for non-trivial changes, so be sure to look down the log for other changes that might be related, implicit commentary.

Last modified 11 years ago Last modified on 08/22/13 13:42:19