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.

Sign up for GitHub

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

Jump to bottom

Remove TreatyOrganization and modify IntergovernmentalOrganization. Fixes #766, #756. #825

Merged
merged 5 commits into from
Apr 28, 2023
Merged

Remove TreatyOrganization and modify IntergovernmentalOrganization. Fixes #766, #756. #825

merged 5 commits into from
Apr 28, 2023

Conversation

hmoore-sa
Copy link
Contributor

@hmoore-sa hmoore-sa commented Apr 25, 2023 •
edited by dylan-sa
Loading

Hi Rebecca, Dylan,

This is a small PR that deprecates the TreatyOrganization class in favor of IntergovernmentalOrganization, adjusts the definition and example of the latter, and makes a pref-label fix to remove a '-'. It also adds the relevant information to the release notes.

Let me know if you don't have time and I can switch reviewers. Aiming for the end of the month.

Thanks,
Heather

Fixes #766, fixes #756.

@hmoore-sa hmoore-sa requested review from rjyounes and dylan-sa April 25, 2023 22:10
@hmoore-sa hmoore-sa linked an issue Apr 25, 2023 that may be closed by this pull request
Remove distinction between Intergovernmental Organization and Treaty Organization #766
Closed
dylan-sa
dylan-sa requested changes Apr 26, 2023
View reviewed changes
Copy link
Contributor

@dylan-sa dylan-sa left a comment

Choose a reason for hiding this comment

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

Just a couple comments on the release note and deprecation file. Otherwise looking good to me.

docs/ReleaseNotes.md Outdated
@@ -18,6 +18,7 @@ Release 11.1.0
- Replaced `rdfs:range` on `gist:conformsTo` with `gist:rangeIncludes`. Issue [#700](https://github.com/semanticarts/gist/issues/700).
- Added the inadvertently omitted predicate `gist:follows`. Issue [#300](https://github.com/semanticarts/gist/issues/300).
- Changed superproperty `gist:startDateTime` to `gist:actualStartDateTime` in formal definition of `gist:ContemporaryEvent`. Issue [#696](https://github.com/semanticarts/gist/issues/696).
- Deprecated `gist:TreatyOrganization` in favor of its parent `gist:IntergovernmentalOrganization` and updated definitions and labels. Issues [#766](https://github.com/semanticarts/gist/issues/766) and [#756](https://github.com/semanticarts/gist/issues/756)
Copy link
Contributor

Choose a reason for hiding this comment

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

For the release note, we should create a separate file containing just the changes related to this PR. Check out what Michael did here, for example: #784.

Can go ahead and delete this line from ReleaseNotes.md.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. And replace "deprecated" with "removed."

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also find the wording "in favor of its parent" a bit strange and would leave it off. This could be broken into two separate items, deprecating one term and fixing the label and definition of the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you both. All suggestions followed. I did not realize about the new file method.

gistDeprecated.ttl Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that the next release will be major and won't contain a deprecation file, no need to add.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed! Thank you, Dylan, and for confiriming that this was -really- ok even though it's in the documentation. :)

@dylan-sa dylan-sa changed the title Feature/issue 776 intergovernmental treaty Remove TreatyOrganization and modify IntergovernmentalOrganization. Fixes #766, #756. Apr 26, 2023
@hmoore-sa hmoore-sa linked an issue Apr 26, 2023 that may be closed by this pull request
Fix prefLabel of IntergovernmentalOrganization #756
Closed
rjyounes
rjyounes requested changes Apr 26, 2023
View reviewed changes
Copy link
Collaborator

@rjyounes rjyounes left a comment

Choose a reason for hiding this comment

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

Everything looks good except the items Dylan flagged. I'll approve once these are fixed.

docs/ReleaseNotes.md Outdated
@@ -18,6 +18,7 @@ Release 11.1.0
- Replaced `rdfs:range` on `gist:conformsTo` with `gist:rangeIncludes`. Issue [#700](https://github.com/semanticarts/gist/issues/700).
- Added the inadvertently omitted predicate `gist:follows`. Issue [#300](https://github.com/semanticarts/gist/issues/300).
- Changed superproperty `gist:startDateTime` to `gist:actualStartDateTime` in formal definition of `gist:ContemporaryEvent`. Issue [#696](https://github.com/semanticarts/gist/issues/696).
- Deprecated `gist:TreatyOrganization` in favor of its parent `gist:IntergovernmentalOrganization` and updated definitions and labels. Issues [#766](https://github.com/semanticarts/gist/issues/766) and [#756](https://github.com/semanticarts/gist/issues/756)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. And replace "deprecated" with "removed."

rjyounes
rjyounes requested changes Apr 26, 2023
View reviewed changes
docs/ReleaseNotes.md Outdated
@@ -18,6 +18,7 @@ Release 11.1.0
- Replaced `rdfs:range` on `gist:conformsTo` with `gist:rangeIncludes`. Issue [#700](https://github.com/semanticarts/gist/issues/700).
- Added the inadvertently omitted predicate `gist:follows`. Issue [#300](https://github.com/semanticarts/gist/issues/300).
- Changed superproperty `gist:startDateTime` to `gist:actualStartDateTime` in formal definition of `gist:ContemporaryEvent`. Issue [#696](https://github.com/semanticarts/gist/issues/696).
- Deprecated `gist:TreatyOrganization` in favor of its parent `gist:IntergovernmentalOrganization` and updated definitions and labels. Issues [#766](https://github.com/semanticarts/gist/issues/766) and [#756](https://github.com/semanticarts/gist/issues/756)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I also find the wording "in favor of its parent" a bit strange and would leave it off. This could be broken into two separate items, deprecating one term and fixing the label and definition of the other.

@hmoore-sa
fixes all changes requested from PR - new release notes file, minor e…
04eea6e 
…dits to release notes, removed class from deprecation file.
@hmoore-sa
Copy link
Contributor Author

Hi @dylan-sa @rjyounes - All requested changes have been made and pushed to the branch. Please take one more look to make sure I've got it all right. And thanks very much, as always, for pointing me in the right direction about how we do things!

rjyounes
rjyounes approved these changes Apr 27, 2023
View reviewed changes
Copy link
Collaborator

@rjyounes rjyounes left a comment

Choose a reason for hiding this comment

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

Looks good!

@rjyounes rjyounes merged commit 85e7dcc into develop Apr 28, 2023
@rjyounes rjyounes deleted the feature/issue-776-intergovernmental-treaty branch April 28, 2023 16:45
@rjyounes rjyounes mentioned this pull request May 2, 2023
Closed
4 tasks
@stevenchalem stevenchalem mentioned this pull request Jun 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylan-sa dylan-sa dylan-sa approved these changes

@rjyounes rjyounes rjyounes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@hmoore-sa @rjyounes @dylan-sa