Fix a bug that --bundle-archive=tar generates .tar.gz instead of a .tar.

Review Request #1707 — Created Feb. 3, 2015 and submitted

jinfeng
pants
1029
9c5159b...
pants-reviews
dturner-tw, ity, jsirois, peiyu

Fix a bug that --bundle-archive=tar generates .tar.gz instead of a .tar.

PANTS_DEV=1 ./pants test tests/python/pants_test/fs

Travis passed: https://travis-ci.org/pantsbuild/pants/builds/49412144

AR
  1. Ship It!
  2. 
      
JI
JS
  1. 
      
  2. Instead of testing the detail - `_ARCHIVER_BY_TYPE` - how about testing the API and use `archiver(typename)` instead.  Less fragile, more to the point.
  3. 
      
JS
  1. An RB style note - the Summary -> 1st line and "fix https://github.com/pantsbuild/pants/issues/1029 and update the tests to catch the problem" is not too helpful to a git log reader or in the CHANGELOG of a release. Since you filled out the bug field the bug link will already be present in the rbt commit -c 1707 commit message - so instead focus the desction on a summary (<=72 cols is a good standard - linux kernel - basically this model: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html) and then if you have more to say you can say it in the description - if not - leave it blank.

    1. done, except a descript is a must have field in rb.

  2. 
      
JI
JS
  1. I can patch this in after CI goes green and you edit the description to just contain that fact in addtiton to your local test line. The data in their now is TMI for the git log and not useful to readers.

    1. erm s/description/tesing done/

  2. 
      
JS
  1. I would highly reccomend you change your pull-request ritual. As things stand you're doing pull requests against your fork instead of against pantsbuild/pants - as a result you do not get travis-ci runs that use the cache. The errors you have been seeing are flaky connection related and these have been largely killed since the cache was enabled.

  2. 
      
JI
JS
  1. LGTM but this does not patch cleanly:

    jsirois@gill ~/dev/3rdparty/pants (master) $ rbt patch -c 1707
    Patch is being applied from request 1707 with diff revision 2.
    
    error: patch failed: tests/python/pants_test/fs/test_archive.py:8
    Falling back to three-way merge...
    Applied patch to 'tests/python/pants_test/fs/test_archive.py' with conflicts.
    U tests/python/pants_test/fs/test_archive.py
    
    The patch was partially applied, but there were conflicts in:
    
        tests/python/pants_test/fs/test_archive.py
    

    You'll need to merge master and upload a new diff.

  2. 
      
JI
JS
  1. Thanks for the fix - this is in master @ https://github.com/pantsbuild/pants/commit/469ccb405714e4a30a870b186f278adaad834776
    Please mark the review as submitted.

  2. 
      
JI
Review request changed

Status: Closed (submitted)

IT
  1. thanks for getting this in so quickly jin, john!

  2. 
      
Loading...