Skip to content

Conversation

@oligot
Copy link

@oligot oligot commented Apr 21, 2013

This patch allows to compile the JSON library with the Gobo Eiffel Compiler:

  • base library: use Gobo FreeELKS version when GOBO_EIFFEL is ge
  • fix a compilation error where the use of a check instruction is not enough for gec (explicitly test that the variable is not Void)

@jvelilla
Copy link
Member

jvelilla commented May 3, 2013

it's ok to me, but, have you run test suites using gec and ise compilers to verify everything is working fine?

@oligot
Copy link
Author

oligot commented May 3, 2013

No, but I will do that and post back when it's done.

@oligot
Copy link
Author

oligot commented May 6, 2013

Test suites are written using AutoTest so I can't test with gec as AutoTest is specific to ISE.
We could use getest instead of AutoTest as getest support both ISE and gec.
What do you think ?

@oligot
Copy link
Author

oligot commented May 7, 2013

Looking at the Git history, I see that @jocelyn removed getest 9 months ago in the commit b6464cf.
The commit message says that this was a duplicate with AutoTest but that's not true: the getest test suites uses Gobo classes (UC_STRING, DS_LIST, DS_HASH_TABLE, ...) in the "model" classes (AUTHOR, BOOK, BOOK_COLLECTION).
Those are valuable tests that could be used to test this PR.

@jvelilla
Copy link
Member

jvelilla commented May 7, 2013

Yes, now that you point it, I remember that those test case were written by
Paul Cohen, and I agree with you they are valuable, but maybe Jocelyn
remove it, because gec was not ready (I mean void safety).

On Tue, May 7, 2013 at 9:29 AM, oligot notifications@github.com wrote:

Looking at the Git history, I see that @jocelynhttps://github.com/jocelynremoved getest 9 months ago in the commit
b6464cfhttps://github.com/eiffelhub/json/commit/b6464cf5e7dbbd82322de59a3c1f87b9bfd2913a
.
The commit message says that this was a duplicate with AutoTest but that's
not true: the getest test suites uses Gobo classes (UC_STRING, DS_LIST,
DS_HASH_TABLE, ...) in the "model" classes (AUTHOR, BOOK, BOOK_COLLECTION).
Those are valuable tests that could be used to test this PR.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-17539052
.

@jocelyn
Copy link
Member

jocelyn commented May 7, 2013

If I remember well, it was not only my decision.
And at that time, the getests were not up to date with ejson, since the
main contributors were using EiffelStudio as main compiler, in order to
minimize the maintenance effort, it was decided to keep only autotest
suites. I was the hand to execute that task.

Now, Olivier, if you are willing to restore those getest test suites and
engage yourself to maintain them, I am fine to have them back in the source
code.

Javier will have the final decision as the project leader, but I think he
also agrees.

On Tue, May 7, 2013 at 2:42 PM, jvelilla notifications@github.com wrote:

Yes, now that you point it, I remember that those test case were written
by
Paul Cohen, and I agree with you they are valuable, but maybe Jocelyn
remove it, because gec was not ready (I mean void safety).

On Tue, May 7, 2013 at 9:29 AM, oligot notifications@github.com wrote:

Looking at the Git history, I see that @jocelyn<
https://github.com/jocelyn>removed getest 9 months ago in the commit
b6464cf<
b6464cf5e7dbbd82322de59a3c1f87b9bfd2913a>

.
The commit message says that this was a duplicate with AutoTest but
that's
not true: the getest test suites uses Gobo classes (UC_STRING, DS_LIST,
DS_HASH_TABLE, ...) in the "model" classes (AUTHOR, BOOK,
BOOK_COLLECTION).
Those are valuable tests that could be used to test this PR.


Reply to this email directly or view it on GitHub<
https://github.com/eiffelhub/json/pull/5#issuecomment-17539052>
.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5#issuecomment-17539603
.

@oligot
Copy link
Author

oligot commented May 8, 2013

Ok, I'll then update this PR to restore the getest test suites.

oligot added 2 commits May 8, 2013 08:56
…e use mainly the later for regression testing)"

This reverts commit b6464cf.
This commit also introduces a new JSON converter for UC_STRING:
JSON_UC_STRING_CONVERTER. This converter is especially used in the JSON
DS_HASH_TABLE converter to retrieve UC_STRING instances instead of STRING_32.

Some more ecf files now use Gobo FreeELKS version when GOBO_EIFFEL is ge.

Note that we can't run the tests using gec for now as the ecf used for Gobo
(which are shipped with EiffelStudio) don't use the Gobo FreeELKS version
(they use the ISE version)
@oligot
Copy link
Author

oligot commented May 8, 2013

Done.
All getest test cases now passes again.

I've also introduced a new JSON converter for UC_STRING: JSON_UC_STRING_CONVERTER.
This converter is especially used in the JSON DS_HASH_TABLE converter to retrieve UC_STRING instances instead of STRING_32.

Some more ecf files now use Gobo FreeELKS version when GOBO_EIFFEL is ge.

Note that we can't run the tests using gec for now as the ecf used for Gobo (which are shipped with EiffelStudio) don't use the Gobo FreeELKS version (they use the ISE version).

oligot added 2 commits May 17, 2013 11:37
We now use Gobo ecf files that use the Gobo FreeELKS version when appropriate.
These ecf files are notably used in json_gobo_extension-portable.ecf and
ejson_test.ecf

The exclude pattern matching has also been changed to support gec (which uses
PCRE).

gec generated files are now ignored by git (in the .gitignore file).
@oligot
Copy link
Author

oligot commented May 17, 2013

All test cases now passes with gec 😏

We now use Gobo ecf files that use the Gobo FreeELKS version when appropriate.
These ecf files are notably used in json_gobo_extension-portable.ecf and
ejson_test.ecf.
For the moment, this point to my personal repository (https://github.com/oligot/gobo-ecf) but this could become a new Eiffelhub repository if the owners are ok...

@jocelyn
Copy link
Member

jocelyn commented May 17, 2013

I don't understand what is the purpose of "gobo-ecf"
Couldn't it be merged into gobo project?

@oligot
Copy link
Author

oligot commented May 17, 2013

There is an open issue but it hasn't been merge yet: gobo-eiffel/gobo#3
I'll ask @ebezault why but in the meantime, I've created a small repository to host these ecf files.

Conflicts:
	library/gobo/converters/json_ds_hash_table_converter.e
	library/json.ecf
@jocelyn
Copy link
Member

jocelyn commented Jan 19, 2015

What is the status on the Pull request?

@oligot
Copy link
Author

oligot commented Jan 20, 2015

Waiting for a new Gobo release before going further...

@jocelyn
Copy link
Member

jocelyn commented Mar 27, 2017

Closing for now, feel free to resubmit if needed.

@jocelyn jocelyn closed this Mar 27, 2017
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 this pull request may close these issues.

4 participants