Conversation
.travis.yml
Outdated
| sudo: false | ||
| python: | ||
| - 2.7 | ||
| # - 3.5 |
.travis.yml
Outdated
| # - 3.5 | ||
| os: | ||
| - linux | ||
| # - osx |
There was a problem hiding this comment.
Why not osx? I actually didn't realise it was possible to use OS X with Travis, but since it evidently is, why wouldn't we want to?
| email: false | ||
|
|
||
| install: true | ||
| script: ./bin/pfs_travis.sh |
There was a problem hiding this comment.
Two comments here:
- It might be nice to add a brief comment to explain what's going on here: to the non-Travis cognoscenti,
install: truedoesn't obviously mean "skip the install step". - Why are we skipping the installation step anyway? Naïvely, one might assume you'd run
install_pfs.shas the install andpfs_tutorial.shas the build (/scriptin this terminology). I assume there are good reasons for not doing so (and I might even learn them as I continue reading), but a brief explanation here would certainly help.
.travis.yml
Outdated
| before_cache: | ||
| - find $HOME/.casher -name "*.tgz" -print -exec rm {} \; | ||
| - rm -r -f $HOME/pfs/pfs | ||
| - rm -r -f $HOME/pfs/tutorial No newline at end of file |
| install: true | ||
| script: ./bin/pfs_travis.sh | ||
|
|
||
| cache: |
There was a problem hiding this comment.
This is another place where a small comment might help. I think the aim here is to cache the directory which has LSST stack installed in it but to clobber the directory with the pfs packages (pfs/pfs) and that with the tutorial outputs (pfs/tutorial), but it's not immediately obvious why we need to prune *.tgz in addition to that, or what the motivation behind the timeout is. As above, I expect a lot of this will become clearer with more reading, but a brief explanation could save some head-scratching.
There was a problem hiding this comment.
Updated following your latest commit — I see the find has gone, but the “bleeding edge casher” doesn't seem to be documented at https://docs.travis-ci.com/user/caching, so I think maybe a little more explanation might still be helpful.
bin/pfs_travis.sh
Outdated
| # If an error occurs, run our error handler to output a tail of the build | ||
| trap 'error_handler' ERR | ||
|
|
||
| # Set up a repeating loop to send some output to Travis. |
There was a problem hiding this comment.
Is this required to prevent Travis from thinking we've timed out, or is it just a comfort display for the user?
bin/pfs_travis.sh
Outdated
| $HERE/bin/install_pfs.sh -b $BUILD_BRANCH $HOME/pfs >> $BUILD_OUTPUT 2>&1 | ||
| . $HOME/pfs/pfs_setups.sh >> $BUILD_OUTPUT 2>&1 | ||
| git lfs install # May not have been done if using cache | ||
| $HERE/bin/pfs_tutorial.sh -b $BUILD_BRANCH -c 4 $HOME/pfs/tutorial >> $BUILD_OUTPUT 2>&1 |
There was a problem hiding this comment.
Why -c 4? Do all the Travis build slaves have four cores?
...actually, according to https://docs.travis-ci.com/user/ci-environment/#Virtualization-environments they have two in this environment (Linux, sudo: false).
bin/pfs_tutorial.sh
Outdated
| # | ||
| # Exercise the PFS 2D pipeline code. | ||
| # | ||
| # We run through the tutorial to make sure everything's working. |
There was a problem hiding this comment.
I wonder if calling this "the tutorial" is helpful in the long term? I understand that for now it's directly derived from the “getting started” guide, but I'd imagine that long term our needs for CI and for user-focused documentation might diverge.
| @@ -0,0 +1,5 @@ | |||
| # Packages should be listed in the order they will be built | |||
| # Each line should contain the repository | |||
| git://github.com/PaulPrice/obs_pfs | |||
There was a problem hiding this comment.
Is your obs_pfs special? Not sure why this doesn't point to Subaru-PFS.
bin/install_pfs.sh
Outdated
| # Install PFS packages | ||
| cd $PREFIX/pfs | ||
| sed -e 's/#.*$//' -e '/^$/d' $HERE/pfs_packages.txt | while read REPO; do | ||
| git_clone $REPO |
There was a problem hiding this comment.
I think you need to be passing $BRANCH false here.
PaulPrice
force-pushed
the
tickets/PIPE2D-83
branch
from
df09ed1 to
af79f08
Compare
This will exercise the pipeline.
We want other packages (e.g., drp_stella, obs_pfs) to run the pipeline under Travis, which involves checking this package out. The travis.sh.TEMPLATE and travis.yml.TEMPLATE provide the necessary machinery to do that. Unfortunately, they need to be copied into the other packages (as travis.sh and .travis.yml). The .TEMPLATE files here in this package will serve as the authoritative version.
Travis have just implemented a feature I requested: deleting the cache after it has been downloaded and extracted. Therefore I can delete my workaround and enable use of the bleeding-edge casher.
PaulPrice
force-pushed
the
tickets/PIPE2D-83
branch
from
af79f08 to
fa7717a
Compare
2 participants
No description provided.