monarch-initiative / mondo-ingest

Coordinating the mondo-ingest with external sources
https://monarch-initiative.github.io/mondo-ingest/
6 stars 3 forks source link

Synchronization: Synonyms - `synonym_type` #639

Closed joeflack4 closed 3 months ago

joeflack4 commented 3 months ago

Merging into:

Overview

Synchronization: Synonyms - synonym_type

Also synonym sync related:

Pre-merge checklist

Documentation

Was the documentation added/updated under docs/?

QC

Was the full pipeline run before submitting this PR using sh run.sh make build-mondo-ingest on this branch (after docker pull obolibrary/odkfull:dev), and no errors occurred?

New Packages

Were any new Python packages added?

Were any other non-Python packages added?

PR Review and Conversations Resolved

Has the PR been sufficiently reviewed by at least 1 team member of the Mondo Technical team and all threads resolved?

Additional information

joeflack4 commented 3 months ago

Reviewed with Trish and approved. We are merging this feature branch into another feature branch. There are still a couple outstanding issues brought up here, but those will be handled in:

twhetzel commented 3 months ago

Joe, we reviewed this PR however I did not officially approve the PR or see the final code updates. This is a bad working process to say someone approved a PR and merge it when it was not approved.

twhetzel commented 3 months ago

Generally looks good. I saw a note the files were re-generated, but still directly under reports vs. reports/sync-synonyms.

joeflack4 commented 3 months ago

@twhetzel Oh, sorry Trish. I asked a couple times if I would merge this PR once I pushed these new changes, and it sounded like you gave verbal confirmation. You probably didn't think I meant it literally.

I'm fine w/ that, though. This is a feature --> feature, so the flow felt different to me, but in the future I'll be sure to follow the same process.

If you want, take a look at the commits I made, and if you add comments/requests on those diffs I will act on them.

reports/syncs-synonyms/ I have in my personal notes for next week, but I've just added a tracker comment; will reply there when done.