Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method header incompatible with STIG #144

Open
cdlm opened this issue Feb 5, 2015 · 10 comments
Open

Method header incompatible with STIG #144

cdlm opened this issue Feb 5, 2015 · 10 comments

Comments

@cdlm
Copy link

cdlm commented Feb 5, 2015

This seems related to #62, and definitely to CampSmalltalk/Cypress#18

FileTree expects method files to look like

category
selector: arg and: arg
    ^ body

But STIG starts the file with a notice/category comment instead of the bare category name before the method code.

@dalehenrich
Copy link
Owner

@cdlm ... technically both formats are correct... FileTree has not been updated to support the newer Cypress format ... I just don't have the time:), especially when the current format is "working just fine." Pull requests are welcome:)

@cdlm
Copy link
Author

cdlm commented Feb 6, 2015

So STIG is the closest to the agreed-upon format?

In fact I'm more concerned about making incompatible changes that will prevent people who already use FileTree from loading their code. I saw there is a notion of layout, would this be a good direction to look into? Do repos/commits have their layout automatically detected?

@dalehenrich
Copy link
Owner

Well, the current FileTree format (first-line method protocol) is also an "agreed-upon" format. The STIG version is a newer format...I'd expect that a platform would support both formats ...

In fact I'm more concerned about making incompatible changes that will prevent people who already use FileTree from loading their code.

as you point out there are quite a few "old-format" repositories out there:) and I share your concerns.

I saw there is a notion of layout, would this be a good direction to look into? Do repos/commits have their layout automatically detected?

For Filetree repos, layout detection should be done by looking at the .filetree property file. In the newer version of Cypress, a property file is explicitly called for, but for Filetree I would expect to retain the .filetree file for backwards compatibility ... Also if you poke around in the code you will find that there are already several prior versions of the Filetree format still supported:

If you're interested in working on new support for the new version I would be inclined to suggest that you start with a fresh set of classes and a fresh approach and not try to reuse a bunch of code ... then the old hierarchy can be easily retired some day:)

@cdlm
Copy link
Author

cdlm commented Feb 11, 2015

On 6 February 2015 at 19:09, Dale Henrichs [email protected] wrote:

If you're interested in working on new support for the new version I would
be inclined to suggest that you start with a fresh set of classes and a
fresh approach and not try to reuse a bunch of code ... then the old
hierarchy can be easily retired some day:)

Which hierarchy ?

I have the FileTree code that is loaded by default in Pharo 4, without unit
tests.

I've looked into loading the tests, but I'm confused about which branch to
load. Trying to load Metacello Preview broke Monticello, and loading the
master branch of FileTree from github seems to rely on broken code (out of
sync with FileSystem I suppose… the error I got was that the #zip method
calls #closed on something that is supposed to be a stream, but is really a
FileReference)

Damien Pollet
type less, do more [ | ] http://people.untyped.org/damien.pollet

@dalehenrich
Copy link
Owner

Ah yes, the load instructions aren't clear... for Pharo3.0 and Pharo4.0, the following:

Metacello new
  baseline: 'FileTree';
  repository: 'github://dalehenrich/filetree:pharo3.0/repository';
  load: 'Tests'.

If you want/need the latest version of Metacello, do the following:

Metacello new
  baseline: 'Metacello';
  repository: 'github://dalehenrich/metacello-work:master/repository';
  get.
Metacello new
  baseline: 'Metacello';
  repository: 'github://dalehenrich/metacello-work:master/repository';
  onConflict: [:ex | ex allow];
  load

These were the classes that I was specifically thinking about junking for the new format...:

MCFileTreeAbstractReader
 MCFileTreeStReader
 MCFileTreeStSnapshotReader
  MCFileTreeStCypressReader
MCFileTreeAbstractStWriter
 MCFileTreeStSnapshotWriter
  MCFileTreeStCypressWriter
 MCFileTreeStWriter

I imagine that the two abstract classes have crift in them to support 5 or 6 different repository formats leading up the present format ... The Cypress in the name refers to the older Cypress format ...

I think that with a fresh start perhaps aimed at supporting both formats and relegating the previous formats to obsucurity (code wise).

The test repositories directory has examples of all of the different formats support by the current hierarcy...

@cdlm
Copy link
Author

cdlm commented Mar 4, 2015

Progress report: I can browse a package coming from VW using the Monticello Browser :)

I installed a new MCCypressRepository to keep the current one useable, and a MCFileTreeReader into which I lazily copy methods from MCFileTreeStCypressReader. I'm extracting smaller methods and factoring stuff one the way… I guess we'll eventually have to check for portability, but at the moment my goal is simplicity and readability of the code.

@cdlm
Copy link
Author

cdlm commented Mar 6, 2015

I've pushed my code as-is on https://github.com/cdlm/filetree/tree/cypress-compatibility-refactor
I'm thinking of opening a pull-request more with the goal of reviewing code than merging, at least in the short term. What do you think ?

@dalehenrich
Copy link
Owner

Yeah, that's not a bad idea ... it will make much easier to manage the code review ... go for it!

@ThierryGoubier
Copy link
Collaborator

I certainly would like to have a look at it :)

@cdlm
Copy link
Author

cdlm commented Mar 6, 2015

See #151.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants