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

Issue #737 sorting concept property values correctly #741

Closed
wants to merge 47 commits into from
Closed

Issue #737 sorting concept property values correctly #741

wants to merge 47 commits into from

Conversation

henriyli
Copy link
Collaborator

Using natural case sort for sorting concept property values when sortByNotation is true, fixes issue #737.

This PR should undo the damage done in the previous bug fix. I had to add an extra parameter for the ConceptProperty constructor. Is this ok or should the vocabulary configuration value be passed some other way?

@henriyli henriyli requested review from osma and kouralex March 20, 2018 15:59
kouralex and others added 27 commits April 9, 2018 12:23
If additional fields are requested using the 'fields' parameter,
add them to the JSON-LD context.

Note: This will break any implementation that relies on the 'skos:'
prefix being part of the keys.
kinow and others added 8 commits June 4, 2018 14:48
The cache that was created in the Model instance, now was moved to the
GlobalConfig, but is still accessed from Model to add entries looked up.

There is no more need of a separate .ttl file for vocabularies, as it is
now part of the configuration file. Old files were removed.

In order to maintain backward compatibility, most methods of GlobalConfig
and of VocabularyConfig were maintained as-is. With internal changes to
re-route requests to the right RDF objects.

Tests were minor updates in tests, but only to replace .inc by .ttl or fix
coverage annotations.
Copy link
Member

@osma osma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the variable name $notsort - it could perhaps be renamed to the more verbose $sort_by_notation - but other than that, looks good!

@osma osma added this to the 2.0 milestone Aug 21, 2018
@osma
Copy link
Member

osma commented Aug 27, 2018

There's a lot of clutter in the commit history. I will do a manual rebase before merging to verify that only the intended changes are included.

@osma
Copy link
Member

osma commented Aug 27, 2018

Merged after manual rebase. Thanks!

@osma osma closed this Aug 27, 2018
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.

6 participants