[hipl-dev] Re: [Merge] lp:~hipl-core/hipl/libhip into lp:hipl

  • From: Miika Komu <mkomu@xxxxxxxxx>
  • To: hipl-dev@xxxxxxxxxxxxx
  • Date: Thu, 23 Feb 2012 20:09:13 +0200

Hi,

On 23/02/12 19:43, Diego Biurrun wrote:
> On Mon, Feb 13, 2012 at 09:32:04AM +0200, Miika Komu wrote:
>> On 02/13/2012 09:14 AM, Diego Biurrun wrote:
>>> On Mon, Feb 13, 2012 at 07:47:31AM +0200, Miika Komu wrote:
>>>> Btw, hipl_autobuild.sh is still semi-automated and a bit tedious to set
>>>> up. It is designed to be run from crontab rather than effortless
>>>> execution from the command line.
>>>
>>> No, no, no, it was never "designed" to be run from cron.  The cronjob
>>> that runs the autobuilder does run from cron, true, but all it does is
>>> call tools/hipl_autobuild.sh from the command line.
>>
>> Why is the emphasis then on the cron execution? Observe the
>> beginning of tools/hipl_autobuild.sh:
>>
>> # HIPL autobuild script for periodic compilation and quality tests.
>> ..
>> # The script is suitable to be run from cron in order to provide basic
> 
> I'll rephrase that to put less emphasis on the cronjob part.  Still my
> point remains valid: I did not intend for this to be run from cron only.
> I tested it from the command line while creating it and I often use it
> from the command line to test my branches.
> 
>>> Previously it would expect a command line parameter to indicate the branch
>>> it was supposed to test, but I recently changed it to derive a suitable
>>> default from Bazaar.  It also used to expect a certain home directory
>>> layout in order to run the OpenWrt and MAEMO cross-compilation tests.
>>> Now it bails out with a message if those are not found, but even before
>>> it would run all the previous tests just fine.
>>
>> Thanks for the history lesson but this doesn't still work out of the box:
>>
>> mkomu@bling:~/projects/hipl-bzr/trunk$ tools/hipl_autobuild.sh
>> usage: tools/hipl_autobuild.sh <branch_name>
>> mkomu@bling:~/projects/hipl-bzr/trunk$ tools/hipl_autobuild.sh trunk
>> bzr: ERROR: Not a branch: "/home/mkomu/src/hipl/trunk/".
>> cat: /home/mkomu/tmp/autobuild/hipl/HIPL_REVISION_trunk: No such
>> file or directory
>>
>> Feel free to say "fix it yourself" or "you're too lazy/stupid/busy
>> to figure to this out" but don't give any excuses why this is
>> perfect as it is because it isn't.
> 
> Should all be fixed.  Thanks for the criticism, let me know what you
> think of the current state.

the current state is awesome! Thanks.

>>>> IMHO, the best way to improve would be to integrate this somehow into
>>>> "make alltests". A better than nothing alternative is to document
>>>> copy-paste-instructions to HACKING explaining "what do I need to
>>>> execute from the command line" to get it working instead forcing all
>>>> developers to read the source code of the script and set it up using
>>>> the trial-and-error method.
>>>
>>> The script has a verbose documentation block at the top.  Duplicating
>>> that I do not consider a good idea, it will only get out of sync.
>>
>> IMHO, the script documents the design but not the usage.
>>
>> Based on this comment, we don't even need the HACKING or manual
>> because all you need is the source code.
> 
> Documentation should not be duplicated.  It is hard enough to keep it
> up to date in one place.  If it is available in two places, I bet
> people will alternate updating either, so both will be broken.
> 
> Please let me know what extra documentation you want and where you
> want to see it.  It should now work out of the box and not need much
> documentation.

Nothing else to complain than the missing chmod a+x
tools/hipl_autobuild.sh :)

Other related posts: