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.

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

Metadata: Add support for default metadata. #65

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

pablinos
Copy link

The current implementation of get for the metadata returns the standard default of an empty string, or empty array, based on the $single parameter.

This change calls get_default_metadata so that any defaults added, along with the hooks, are respected.

The current implementation of `get` for the metadata returns the
standard default of an empty string, or empty array, based on the
`$single` parameter.

This change calls `get_default_metadata` so that any defaults added,
along with the hooks, are respected.
@pablinos
Copy link
Author

I've kept this as a draft for now as it needs some tests, and it might be worth combining it with #53 to make get_metadata behave more like production.

@leogermani
Copy link
Collaborator

Thanks for that.

This all makes sense, we just need to find some time to write some tests for both PRs and make sure we cover the behavior we want.

I'll try to do this as soon as I can

@szepeviktor
Copy link
Collaborator

Maybe someone jumps in and contributes unit tests.

@pablinos
Copy link
Author

This all makes sense, we just need to find some time to write some tests for both PRs and make sure we cover the behavior we want.

Yes definitely. I'll try and make some time for it too. The PR I was working on that would have needed this change ended up going in a different direction, but it's probably still worth us getting the metadata API working as close to production as possible. If I (almost) needed it once, I'm sure it'll come up again!

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.

3 participants