Being friendly to quick contributions

General
1

NOTE: I moved this post from Improving contributor guide discoverability (was: Consolidating the 71 GitHub repositories to simplify maintenance and contribution) - #10 by jan because I think it is a different matter (albeit related) that should be in a separate thread.

The normal quick contributor just would like to do a quick patch of the documentation, or some small bug with a one-line change, etc. The first thing that user would do is to fork the swipl-devel repo in his github account. But we make it very difficult after that.

Here is what happens to that contributor who has forked swipl-devel in his github user account:

$ git clone https://github.com/someuser/swipl-devel
Cloning into 'swipl-devel'...
remote: Enumerating objects: 184191, done.
remote: Total 184191 (delta 0), reused 0 (delta 0), pack-reused 184191
Receiving objects: 100% (184191/184191), 80.62 MiB | 3.26 MiB/s, done.
Resolving deltas: 100% (147549/147549), done.
$ cd swipl-devel
$ git submodule update --init
Submodule 'bench' (https://github.com/erlanger/bench.git) registered for path 'bench'
Submodule 'debian' (https://github.com/erlanger/distro-debian.git) registered for path 'debian'
Submodule 'packages/PDT' (https://github.com/erlanger/packages-PDT.git) 
[....submodule registration...]
Submodule 'packages/zlib' (https://github.com/erlanger/packages-zlib.git) registered for path 'packages/zlib'
Cloning into '/tmp/swipl-devel/bench'...
Username for 'https://github.com':     <<<--------- LOOK HERE

Uhh? It is asking for the user name? The user who just wants to make a one-line change will simply say: “why is it asking me for the user name? This is too hard I’ll do it sometime later”, the end result: we’ll never get the contribution.

The more persistent user will start googling around, and figure out that it is asking for the user name because of the way .gitmodules is set up. Then he will figure out an hour later, that he has to change .gitmodules the way it is described in this PR. This is why travis can’t build SWI-Prolog without the patch in the PR.

The reason why Jan has never experienced this is because he is the owner of the repo.

Jan, you would see the above if you fired up a VM, fork swipl-devel from a new github account, and try to make a one line patch as if you were not the author of the project.

2

There is another way with patches and without GitHub-forking. This is the original way used by the first Git users before there were GitHub.

  1. git clone https://github.com/SWI-Prolog/swipl-devel
  2. git clone https://github.com/SWI-Prolog/packages-ssl (example)
  3. Write the contribution and commit it.
  4. In swipl-devel, git format-patch --stdout COMMIT^ | xclip -in -sel clipboard
  5. Paste the patches to a maintainer.
  6. In packages-ssl, git format-patch --stdout COMMIT^ | xclip -in -sel clipboard
  7. Paste the patches to a maintainer.
  8. The maintainer pastes the patch into git am.

Unfortunately few people take the time to learn Git (they have work to do; and Git is not totally user-friendly either).

Thus, one way to be friendly to quick contributions is to require the minimum Git knowledge from the contributors.

3

The git format-patch way is pretty ok with me, although the discussion options with pull requests are sometimes useful. If patches get abandoned for now (immature, incomplete, IMO wrong direction), a pull request allows others to continue with them as they please. So, both mail and PRs have their merit.

Contributing using PRs isn’t too hard though, especially as most people will contribute only to the core and/or one or two modules. It works as follows:

git clone https://github.com/SWI-Prolog/swipl-devel
git submodule update --init

Now build the system and hack around. At any time (either before or after the hacking, if you want to create (for the first time) a pull request for module X:

<clone it on github to say the user `myself`>
cd packages/X                                # or wherever the module is
git remote add myfork git@github.com:myself/X.git # or https://, at your preference

Now assuming you did the usual stuff to create a topic branch relative to master, use

git push myfork mytopic:mytopic

Just for completeness, for creating a topic branch:

cd packages/X
git checkout master
git pull
git checkout -b mytopic
<hack around and commit>

Note that besides the things you need to do anyway, there are just a few extra steps:

  • git submodule update --init
    To get the submodules and update them after pulling the main repo.
  • git checkout master
    git pull
    To get the submodule on a branch and in sync with the main repo

I advice to activate the bash completion and extensions in your shell when working with git. It tells you where you are and what the status is and reduces the typing a lot.

4

Let’s keep using submodules and forget about subtrees for now, for the reasons Jan mentioned in the other thread (XSB cooperation, multi-authorship, etc.). After trying submodules myself, I find that submodules aren’t too hard. The key is to clone before forking, not to fork before cloning. It seems that most confusion can be avoided by telling people about that expected order of operations.

By the way, I was trying to link to Jan’s reply above from the How to submit patches? page, so I clicked Edit this page, but it throws an exception:

goal unexpectedly failed: wiki_edit:wiki_edit([protocol(http),peer(ip(10,0,3,1)),pool(client('httpd@80',plweb:http_dispatch,<stream>(0x7f256c00b3f0),<stream>(0x7f256c003570))),input(<stream>(0x7f256c00b3f0)),method(get),request_uri('/wiki_edit?location=/howto/SubmitPatch.html'),path('/wiki_edit'),search([location='/howto/SubmitPatch.html']),http_version(1-1),host('eu.swi-prolog.org'),x_forwarded_for('175.158.50.112'),connection(close),upgrade_insecure_requests('1'),user_agent('Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.96 Safari/537.36'),accept([media(text/html,[],1.0,[]),media(application/'xhtml+xml',[],1.0,[]),media(image/webp,[],1.0,[]),media(image/apng,[],1.0,[]),media(application/xml,[],0.9,[]),media(_2088/_2090,[],0.8,[])]),referer('http://www.swi-prolog.org/howto/SubmitPatch.html'),accept_encoding('gzip, deflate, br'),accept_language('en-US,en;q=0.9,id;q=0.8')])
In:
[13] throw(error(goal_failed(...),_2170))
[12] http_dispatch:call_action(wiki_edit:wiki_edit,[protocol(http),...|...]) at /home/swipl/lib/swipl/library/http/http_dispatch.pl:1007
[10] time:run_alarm_goal('$alarm'(11365602910596),http_dispatch:call_action(...,...,...)) at /home/swipl/lib/swipl/library/time.pl:145
[9] setup_call_catcher_cleanup(time:alarm(300,...,...,...),time:run_alarm_goal(...,...),_2306,time:remove_alarm_notrace(...)) at /home/swipl/lib/swipl/boot/init.pl:468
[5] httpd_wrapper:call_handler(http_dispatch:time_limit_action(...,...,...),2359296,-) at /home/swipl/lib/swipl/library/http/http_wrapper.pl:314
[4] catch(httpd_wrapper:call_handler(...,2359296,-),error(goal_failed(...),context(_2450,_2452)),httpd_wrapper:true) at /home/swipl/lib/swipl/boot/init.pl:386
[3] httpd_wrapper:handler_with_output_to(http_dispatch:time_limit_action(...,...,...),2359296,-,current_output,error(goal_failed(...),context(_2524,_2526))) at /home/swipl/lib/swipl/library/http/http_wrapper.pl:297

Note: some frames are missing due to last-call optimization.
Re-run your program in debug mode (:- debug.) to get more detail.
5

There seems to be something missing here, for contributing to packages (e.g., packages/clib). Do I need to create clones for all the packages? [I gave up figuring this out and sent a patch instead] NB: My knowledge of git is summed up by https://xkcd.com/1597/

Also, it seems that the modern way to clone with submodules is git clone --recurse-submodules, with no need for git submodule update --init) … I would have submitted a documentation fix for https://www.swi-prolog.org/howto/SubmitPatch.html except I couldn’t find it in the source tree.

6

Thanks for the patch. Applied. To make a pull request for a package,

  • Clone the main repo (swipl-devel.git)
  • Init the packages (using one of the ways you describe)
  • Go to the package
  • run git checkout master && git pull
  • branch: git checkout -b myfix
  • Hack away and commit

So far all is pretty normal. Now we come to the special part …

  • Fork the package at github, creating e.g. git@github.com:me/packages-clib.git
  • Run (from the clib checked out dir)
    • git remote add fork git@github.com:me/packages-clib.git
    • git push fork myfix:myfix

Now create a PR at github.

But, good old git patches are just as easy to handle for me, so what you did is perfectly fine.

The website content is in the repo plweb-www.git

7

For what it’s worth, I set out this weekend to see these instructions instructions were clear enough for me to follow. The verdict is: yes, they are.

I stumbled a bit from time to time, but believe I have managed to build SWIPL locally from source, find a small flaw in packages/cmake/PackageDoc.cmake, fix it, test the fix, push it to my fork on Github, and make a pull request on Github. (That said, I sympathize with the desire to make it friendlier, though I don’t know how. Being able to fork the repo before cloning would not have made an iota of difference to me. Working with unfamiliar tools – and for me that includes Github – is always a challenge if things go wrong, and I was kept going in large part by reflecting on the long years I have been using swipl and the huge debt of gratitude I owe the project.)

3 Likes