lexonomy
/
lexonomy_custom_editor
7
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity
Labels Milestones

mt-homonymy-support #2

Manually merged
lexonomy merged 7 commits from mt-homonymy-support into master 3 years ago
Conversation 3 Commits 7 Files Changed 12
matic_t commented 3 years ago
Collaborator
There is no content yet.
ozbolt was assigned by matic_t 3 years ago
lkavcic was assigned by matic_t 3 years ago
matic_t added 4 commits 3 years ago
matic_t commented 3 years ago
Poster
Collaborator

Probably best if both of you check it out.
Ožbolt for prevention and Luka so we both can speed up understanding of whole stack.

Basically we're adding support for the example in vsms1.xml file. It's already in schema so we just had to provide support in editor.

https://lexonomy.cjvt.si/5pdds79p/ - link to plugin used on dictionary

Probably best if both of you check it out. Ožbolt for prevention and Luka so we both can speed up understanding of whole stack. Basically we're adding support for the example in vsms1.xml file. It's already in schema so we just had to provide support in editor. https://lexonomy.cjvt.si/5pdds79p/ - link to plugin used on dictionary
matic_t added 1 commit 3 years ago
matic_t added 1 commit 3 years ago
matic_t added 1 commit 3 years ago
ozbolt commented 3 years ago
Collaborator

First of all, I did not test the code, I just went over the changes.

About the code, there are two mistakes:

  • homonomy_list_getter in common_accessors is only used once. This is fine, but the way I imagined it is that if this kind of functionality is only used once, than implement it inside the class (this time its EditHomonymy). Then later if needed to avoid code duplication refactor stuff out of the class and in common_accessors. But that is fairly subjective point and you can decide to do it another way :)
  • second one, if your IDE does white spaces this way, than create a big commit with only white space changes and than work from there. This is again not so important, but makes git blame and commits themselfs much less readable.

Otherwise, yes, everything looks nice and clean, gg :)

First of all, I did not test the code, I just went over the changes. About the code, there are two mistakes: * `homonomy_list_getter` in `common_accessors` is only used once. This is fine, but the way I imagined it is that if this kind of functionality is only used once, than implement it inside the class (this time its `EditHomonymy`). Then later if needed to avoid code duplication refactor stuff out of the class and in common_accessors. But that is fairly subjective point and you can decide to do it another way :) * second one, if your IDE does white spaces this way, than create a big commit with only white space changes and than work from there. This is again not so important, but makes git blame and commits themselfs much less readable. Otherwise, yes, everything looks nice and clean, gg :)
lkavcic commented 3 years ago
Collaborator

ok

ok
lexonomy merged commit a07b35c6a8 into master manually 3 years ago
The pull request has been manually merged as a07b35c6a8.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
Labels
Apply labels
Clear labels
No items
No Label
Milestone
Set milestone
Clear milestone
No items
No Milestone
Assignees
Assign users
Clear assignees
No Assignees
ozbolt
lkavcic
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lexonomy/lexonomy_custom_editor#2
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'mt-homonymy-support'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes