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

containedIn (between Places) is named in the most annoying direction. Add inverseOf, containsPlace #811

Closed
danbri opened this Issue Sep 29, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@danbri
Contributor

danbri commented Sep 29, 2015

Let's rename containedIn to containedInPlace too.

@mfhepp

This comment has been minimized.

Show comment
Hide comment
@mfhepp

mfhepp Sep 29, 2015

Contributor

I am fine with that, but I do not think it is too annoying as it stands ;-)
Note that we are using containedIn in the hotel proposal, so there is a potential merge conflict if you change the name at the time of adding my proposal.

Contributor

mfhepp commented Sep 29, 2015

I am fine with that, but I do not think it is too annoying as it stands ;-)
Note that we are using containedIn in the hotel proposal, so there is a potential merge conflict if you change the name at the time of adding my proposal.

@danbri

This comment has been minimized.

Show comment
Hide comment
@danbri

danbri Sep 29, 2015

Contributor

Thanks, ack'd.

Contributor

danbri commented Sep 29, 2015

Thanks, ack'd.

danbri added a commit that referenced this issue Sep 29, 2015

Fix for #811 - gave containedIn a more Place-related name, and an inv…
…erse.

Since most markup using this will start with the larger, outer place, this was long overdue.

@danbri danbri self-assigned this Sep 29, 2015

@danbri danbri added this to the sdo-phobos release milestone Sep 29, 2015

@pmika

This comment has been minimized.

Show comment
Hide comment
@pmika

pmika Sep 30, 2015

Collaborator

+1

Collaborator

pmika commented Sep 30, 2015

+1

@scor

This comment has been minimized.

Show comment
Hide comment
@scor

scor Sep 30, 2015

Contributor

👍

Contributor

scor commented Sep 30, 2015

👍

@chaals

This comment has been minimized.

Show comment
Hide comment
@chaals

chaals Sep 30, 2015

Contributor

Looks OK to me, but again, providing crossrefs between containedInPlace and containsPlace seems sensible

Contributor

chaals commented Sep 30, 2015

Looks OK to me, but again, providing crossrefs between containedInPlace and containsPlace seems sensible

@danbri

This comment has been minimized.

Show comment
Hide comment
@danbri

danbri Sep 30, 2015

Contributor

Thanks @chaals crossrefs in this case are built in, since we declare them to be inverses of each other - see links above

Contributor

danbri commented Sep 30, 2015

Thanks @chaals crossrefs in this case are built in, since we declare them to be inverses of each other - see links above

@danbri

This comment has been minimized.

Show comment
Hide comment
@danbri

danbri Oct 30, 2015

Contributor

Queued for closure after next release. http://sdo-phobos.appspot.com/containedInPlace

Contributor

danbri commented Oct 30, 2015

Queued for closure after next release. http://sdo-phobos.appspot.com/containedInPlace

@danbri

This comment has been minimized.

Show comment
Hide comment
@danbri
Contributor

danbri commented Nov 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment