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

adopt format agreed with Gemstone #52

Closed
estebanlm opened this issue Sep 26, 2018 · 2 comments · Fixed by #100
Closed

adopt format agreed with Gemstone #52

estebanlm opened this issue Sep 26, 2018 · 2 comments · Fixed by #100
Assignees
Milestone

Comments

@estebanlm
Copy link
Contributor

During ESUG (2017, so more than a year) we agreed a format for Tonel files that is slightly different from the one existing now.
We agreed:

  • Left side of ston declarations (keys) will go in symbols.
  • Right side will NOT be symbols. They will be plain strings and any transformation into symbols needs to be solved in image.

This easies some parsing, etc.

@gcotelli
Copy link
Contributor

I suppose this is not implemented yet because it will make every package in Pharo change on every commit when these changes got merged.
So I will propose to do the following:

  • Add some metadata to .properties file to indicate the tonel format version, and defaulting to the current behavior in case a version is not specified. Eg:
    {
    #format : 'tonel',
    #formatVersion: '2.0'
    }
    
  • Use this information to decide which version of the tonel writer to use
  • Change the first-time creation of the properties file to use the newest format version available, so new projects will start with the most updated one.

This way projects wanting to use the new format features can adopt them without disrupting projects on prior versions.
And paves a way for the future evolution of the format.

I think in the case of Tonel we need to apply the Robustenss principle, and produce an output that is as deterministic as possible (via the TonelWriter) but be more lenient on the input (via the TonelReader).

If we must produce a new format version I will say we need to consider #99 also.

@tesonep @estebanlm @dalehenrich @eMaringolo @guillep what do you think?

@dalehenrich
Copy link

Most of the tonel readers will read any format and covert to the "correct" format internally (I think that is part of the tonel spec as well), but all tonel writers should write in the agreed upon format moving forward ... The longer that we wait to start writing the correct format, the bigger the problem will become ... So bite the bullet and fix the problem now and provide patches for all of the active versions of Pharo that are using tonel, so that the issues will be mitigated in short order ...

I just noticed that Pharo (randomly?) writes mixed Strings and Symbols as values ...this was written 8 days ago and this was written 19 days ago ... hint look at the category field

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

Successfully merging a pull request may close this issue.

4 participants