Skip to content
This repository was archived by the owner on Oct 4, 2022. It is now read-only.

Conversation

hannaw93
Copy link
Contributor

@hannaw93 hannaw93 commented Oct 16, 2019

Summary

This PR can be summarized in the following changelog entry:

  • [yoastseo] Checks whether a word is one of the exceptions for which we list all of the forms and the canonical stem. If it matches one of the listed forms, only return the stem.

Relevant technical choices:

  • The code is made as a morphoHelper function as it can be reused later for different language.

Test instructions

This PR can be tested by following these steps:

  • Add more words in spec file.
    The words can be taken from stemmingExceptionStemsWithFullForms list in the morphology data file. E.g. "zijden" from this list.
    ["zij", ["zij", "zijde", "zijden", "zijdes", "zitje", "zijtjes"]]
    The expected result of the test: the stem (i.e. "zij")

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended

Fixes #371
Fixes https://yoast.atlassian.net/browse/LIN-103

@hannaw93 hannaw93 changed the base branch from develop to feature/Dutch-morphology October 16, 2019 13:36
Copy link
Contributor

@agnieszkaszuba agnieszkaszuba left a comment

Choose a reason for hiding this comment

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

CR: needs changes

Looks good, with the exception of a function that could be moved to helpers and be used for both Dutch and German. See my comment :)

Copy link
Contributor

@agnieszkaszuba agnieszkaszuba left a comment

Choose a reason for hiding this comment

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

CR: needs changes

I have added an extra spec to test that when the preceding lexical material is shorter than 2, null is returned.

I have also added some comments about naming and descriptions, as well as a line of code which I think needs to be removed.

Could you also make sure the the PR description and testing instructions are up to date when submitting an issue for code review? :)

@nataliashitova nataliashitova added this to the Dutch morphology milestone Feb 11, 2020
@nataliashitova nataliashitova merged commit 26ceb7f into feature/Dutch-morphology Feb 11, 2020
@nataliashitova nataliashitova deleted the 371-exceptions-full-forms branch February 11, 2020 15:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants