Merge lp://staging/~alex-idereal/bzr-java-lib/fetch-xmloutput-during-build into lp://staging/bzr-java-lib

I like the idea of automating bzr-java-lib build very much, however, I see a couple of issues with the proposed implementation.

1) BZR_PLUGIN_PATH env variable should not be overwritten by Java code or in any other way if the variable is already set by the user.
Currently setting BZR_PLUGIN_PATH pointing to any folder (e.g. Eclipse workspace with bzr-xmloutput) makes it really easy to run bzr-java-lib tests against different versions of xmloutput and check backward compatibility for instance. I would like to see some check before an attempting to discover xmloutput location and setting the variable.

2) The scripts (fetch-bazaar-plugins.bat, fetch-bazaar-plugins.sh) are inconsistent.
IMHO they should do the same regardless of OS i.e. create a local folder and check out xmloutput source code from launchpad. I am not sure if the script should do anything if BZR_PLUGIN_PATH is already set or not.

3) Scripts location. "bin" is usually considered the default output folder location in Eclipse when maven is not used. It might be a bit confusing for some developers to see some scripts there :) Please consider moving the scripts to root folder or rename "bin" to "scripts" or something similar.

Agreed on all points.

Unfortunately I don't have easy access to DOS/Windows to test the
DOS script, so if someone could help me? I guess I can try WINE.

  > Review: Needs Fixing

  > I like the idea of automating bzr-java-lib build very much, however, I see
  > a couple of issues with the proposed implementation.

  > 1) BZR_PLUGIN_PATH env variable should not be overwritten by Java code or
  > in any other way if the variable is already set by the user. Currently
  > setting BZR_PLUGIN_PATH pointing to any folder (e.g. Eclipse workspace with
  > bzr-xmloutput) makes it really easy to run bzr-java-lib tests against
  > different versions of xmloutput and check backward compatibility for
  > instance. I would like to see some check before an attempting to discover
  > xmloutput location and setting the variable.

  > 2) The scripts (fetch-bazaar-plugins.bat, fetch-bazaar-plugins.sh) are
  > inconsistent. IMHO they should do the same regardless of OS i.e. create a
  > local folder and check out xmloutput source code from launchpad. I am not
  > sure if the script should do anything if BZR_PLUGIN_PATH is already set or
  > not.

  > 3) Scripts location. "bin" is usually considered the default output folder
  > location in Eclipse when maven is not used. It might be a bit confusing for
  > some developers to see some scripts there :) Please consider moving the
  > scripts to root folder or rename "bin" to "scripts" or something similar.

--
idereal | deliver http://idereal.co.nz/
Get your software to market sooner 022 659 0282

I have pushed some more changes to address the raised points.

--
idereal | deliver http://idereal.co.nz/
Get your software to market sooner 022 659 0282

Great, the changes look good. I have a couple of more comments:

1) I am not sure if the shell scripts should handle changes in xmloutput source code or not. Currently they cannot. I would expect the scripts to check out a fresh copy of xmloutput on each run, however, the Linux one performs the following check: [[ -d xmloutput ]] && exit. On the other hand, the scripts will do their job for newly created branches and they can be easily changed in the future if needed

2) The path in pom.xml requires a correction:
<executable>${basedir}/bin/${fetch-bazaar-plugins}</executable>
should be:
<executable>${basedir}/scripts/${fetch-bazaar-plugins}</executable>

3) Could you please consider changing the license to LGPL?

Cheers,
Piotr

  > 1) I am not sure if the shell scripts should handle changes in xmloutput
  > source code or not. Currently they cannot. I would expect the scripts to
  > check out a fresh copy of xmloutput on each run, however, the Linux one
  > performs the following check: [[ -d xmloutput ]] && exit. On the other
  > hand, the scripts will do their job for newly created branches and they can
  > be easily changed in the future if needed

I have updated it to check the version and replace if necessary,
I think this covers these two scenarios:

 + Development involves changes to xmloutput.
 + Standard version of xmloutput is fine.

For the DOS script we should probably change it to delete the
entire tree and export from launchpad each time. Could you
validate that script for me?

  > 2) The path in pom.xml requires a correction:
  > <executable>${basedir}/bin/${fetch-bazaar-plugins}</executable> should be:
  > <executable>${basedir}/scripts/${fetch-bazaar-plugins}</executable>

Fixed.

  > 3) Could you please consider changing the license to LGPL?

I updated it to GPL v2 or later to match the LICENSE file in bzr-java-lib.

Alex

--
idereal | deliver http://idereal.co.nz/
Get your software to market sooner 022 659 0282

The patch looks good. I would like to change bzr-java-lib licence to LGPL at some point, but it might be actually a good idea to change it consistently everywhere at the same time.

Regarding the DOS script, here is what I have come up with:
IF EXIST xmloutput (
   rd /S /Q xmloutput
)
IF EXIST ..\bzr-xmloutput (
   robocopy ..\bzr-xmloutput xmloutput *.* /MIR
) ELSE IF EXIST ..\..\bzr-xmloutput (
   robocopy ..\..\bzr-xmloutput xmloutput *.* /MIR
) ELSE IF EXIST ..\..\..\bzr-xmloutput (
   robocopy ..\..\..\bzr-xmloutput xmloutput *.* /MIR
) ELSE (
   bzr export -r tag:0.8.8 xmloutput lp:bzr-xmloutput
)