Skip to content

Conversation

@cdhorn
Copy link
Collaborator

@cdhorn cdhorn commented Nov 9, 2020

This removes ?all parm and adds /all in path as response formats differed which was not clean. Includes unit tests and apispec.yaml update.

Now wondering if I should have just added this to PR #60

@DavidMStraub
Copy link
Member

Looks good!

Concerning locale, I guess this is can later be promoted to use a locale parameter on the base class's get, right?

Could you add a unit test for locale, too? This would ease refactoring as part of #56.

@cdhorn
Copy link
Collaborator Author

cdhorn commented Nov 14, 2020

Concerning locale, I guess this is can later be promoted to use a locale parameter on the base class's get, right?

Probably, but this might need to get refactored again at some point, I am unsure. I did not realize before but if I recall there appear to be plugin relationship calculators for different languages. I'm not sure how this and they are related.

Could you add a unit test for locale, too?

I will look at it.

@cdhorn
Copy link
Collaborator Author

cdhorn commented Nov 14, 2020

Rebased. Unsure if locale test I added should do anything more specific than it is.

@DavidMStraub
Copy link
Member

Perfect.

@DavidMStraub DavidMStraub merged commit 76ab182 into gramps-project:master Nov 14, 2020
@cdhorn cdhorn deleted the relation-tests branch November 25, 2020 20:17
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.

2 participants