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

adding Healey 1970 #1311

Merged
merged 5 commits into from
Apr 16, 2023
Merged

adding Healey 1970 #1311

merged 5 commits into from
Apr 16, 2023

Conversation

SimonGreenhill
Copy link
Contributor

Pull request checklist

  • add new concept list
  • add new metadata
  • add new Concepticon concept sets
    • checked whether the new concept(s) can be applied to existing lists with
      concepticon notlinked --gloss "NEW_GLOSS"
  • add new Concepticon concept relations
  • refine existing Concepticon concept set mappings
  • refine Concepticon glosses
  • refine Concepticon concept relations
  • refine Concepticon concept definitions
  • retire data

Additional information

New concept list, scanned list attached here:

Healey1970.pdf

@AnnikaTjuka
Copy link
Collaborator

@SimonGreenhill I'll moderate the PR after the release. I hope it's okay if we don't include the list in 3.1.

Copy link
Collaborator

@MuffinLinwist MuffinLinwist left a comment

Choose a reason for hiding this comment

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

@AnnikaTjuka I was just finishing writing my review. So maybe it can stay there until your revision after the release.

concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
@SimonGreenhill
Copy link
Contributor Author

Thanks @MuffinLinwist -- most changes made, except for the ones I commented on.

@AnnikaTjuka
Copy link
Collaborator

Thanks, @MuffinLinwist and @SimonGreenhill! Would you have time to do the second review for the list, @Tarotis?

Copy link
Collaborator

@FredericBlum FredericBlum left a comment

Choose a reason for hiding this comment

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

Sure, thanks for adding the list @SimonGreenhill , here are my comments. I have added a couple of suggestions for changes, and also one case where I'd ask for your suggestion @AnnikaTjuka .

concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
@AnnikaTjuka
Copy link
Collaborator

@Tarotis I just went through all your comments. I'd generally refrain from changing the glosses if they are in the original list. If we notice a lot of mistakes in a list, we usually add an extra column where we correct the glosses. But it must be transparent that we are making the changes and that they do not correspond to the original list.

@SimonGreenhill
Copy link
Contributor Author

worked through @Tarotis suggestions.

@AnnikaTjuka
Copy link
Collaborator

Thanks, @SimonGreenhill! I'll do a final check later today.

Copy link
Collaborator

@AnnikaTjuka AnnikaTjuka left a comment

Choose a reason for hiding this comment

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

Just three more suggestions but then everything should be ready to be merged. @Tarotis We'll need your approval of the requested changes before merging.

concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
concepticondata/conceptlists/Healey-1970-364.tsv Outdated Show resolved Hide resolved
@SimonGreenhill
Copy link
Contributor Author

all Annika's changes have been made.

Copy link
Collaborator

@AnnikaTjuka AnnikaTjuka left a comment

Choose a reason for hiding this comment

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

Many thanks, @SimonGreenhill! Feel free to merge.

@SimonGreenhill SimonGreenhill merged commit 207810e into master Apr 16, 2023
@SimonGreenhill SimonGreenhill deleted the h70 branch April 16, 2023 08:52
@SimonGreenhill
Copy link
Contributor Author

thanks Annika, merged.

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.

None yet

4 participants