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

refactor Sense.PartOfSpeech to use an object #1232

Closed
hahn-kev opened this issue Nov 13, 2024 · 1 comment · Fixed by #1350
Closed

refactor Sense.PartOfSpeech to use an object #1232

hahn-kev opened this issue Nov 13, 2024 · 1 comment · Fixed by #1350
Assignees
@rmunn
Labels
📖 MiniLcm issues related to miniLcm library code, includes fwdat bridge and lcmCrdt

Comments

@hahn-kev
Copy link
Collaborator

Right now Sense.PartOfSpeech has 2 properties, one for the Id and one for the English text, I'd like to refactor this to just use the part of speech object. Keep in mind this will also require changes on the frontend too
@hahn-kev hahn-kev mentioned this issue Nov 13, 2024
Merged
@hahn-kev hahn-kev added the 📖 MiniLcm issues related to miniLcm library code, includes fwdat bridge and lcmCrdt label Dec 11, 2024
@rmunn
Copy link
Contributor

rmunn commented Jan 7, 2025

Part of the fixes for #1279 in PR #1344 would be improved if this was done. I had to comment out the checks for PartOfSpeechId because it was impossible to tell if the part of speech was meant to be canonical or user-created. Once senses reference PartOfSpeech objects directly, then some of the commented-out code in PR #1344 will be safe to uncomment.

@rmunn rmunn mentioned this issue Jan 7, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rmunn rmunn

Labels
📖 MiniLcm issues related to miniLcm library code, includes fwdat bridge and lcmCrdt
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make Sense.PartOfSpeech an object instead of a string sillsdev/languageforge-lexbox
2 participants
@rmunn @hahn-kev