search

dalehenrich / filetree

Monticello repository for directory-based Monticello packages enabling the use of git, svn, etc. for managing Smalltalk source code.
https://github.com/CampSmalltalk/Cypress
MIT License
133 stars 26 forks source link

quote packageDirectory to handle package-names which contain spaces #140

Closed eMBee closed 9 years ago

eMBee commented 9 years ago

this is my first attempt at a contribution via gitfiletree. should i be removing all those property changes? it looks very messy.

dalehenrich commented 9 years ago

@eMBee, thanks for your contribution!

The tests with your changes aren't passing for Pharo4.0:

**************************************************************************************
    Results for #('BaselineOfFileTree') Test Suite
46 run, 45 passes, 0 skipped, 0 expected failures, 0 failures, 0 errors, 1 unexpected passes
**************************************************************************************
*** FAILURES *******************
    MCFileTreeLoaderTraitsTest debug: #testLoad.
**************************************************************************************

Pharo4.0 is a bit of a moving target these days, so it isn't always obvious if the test failure is due to your changes or changes in Pharo4.0 that are independent of your changes ... I had run a test earlier today with some of my own changes that failed running Pharo4.0, so I am suspicious that the root problem is in Pharo4.0 ...

Since the changes are in @ThierryGoubier code, I will leave it up to him to review your work with relation to the failed tests ...

At the end of the day, I think may end up creating a Pharo4.0 branch if changes need to be made on the filetree side of things ...

ThierryGoubier commented 9 years ago

Hi @eMBee, the property changes are expected, don't worry about them. I'll have a look on the tests part of things.

dalehenrich commented 9 years ago

Oops @ThierryGoubier, I inadvertantly restarted the pull request trvis job ... meant to restart my failed tests ... sorry

dalehenrich commented 9 years ago

@ThierryGoubier I'm tracking a slightly different problem with Pharo4.0 for my work on Issue #136 ... I will wait until you are done merging this pull request into the pharo3.0 before merging my work ... I might end up having to create a separate pharo4.0 branch, dependeing upon what I find...hopefully I won't have to...

ThierryGoubier commented 9 years ago

Ok, the failing test is an unexpected pass :)

We need a pharo4.0 branch and correct there, otherwise we won't get a green travis. Or I merge this on the pharo3.0 branch first? It's green on 3.0 and we start the work on the pharo4.0 branch from that point.

dalehenrich commented 9 years ago

@ThierryGoubier I see that this is a pragma, if the #expectedFailures method still works for unit tests,then we don't need a new branch ... yet ... I'm tracking another Pharo4.0 issue, so I will take care of this one way or the other ... if you are comfortable with this change go ahead and complete the merge and I will deal with the pharo3.0 vs pharo4.0 branch separately ...