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

Resource Identifier metadata #206

Closed
wants to merge 7 commits into from
Closed

Resource Identifier metadata #206

wants to merge 7 commits into from

Conversation

zachdaniel
Copy link

@zachdaniel zachdaniel commented Dec 19, 2016

Attempting to address #202

@zachdaniel zachdaniel changed the title Add meta with configuration Resource Identifier metadata Dec 19, 2016
%{data: source_data, conn: conn},
%{identifer_meta: identifier_metadata_func})
when is_function(identifier_metadata_func) do
identifier_metadata_func.(source_data, destination_data, conn)
Copy link
Contributor

@alanpeabody alanpeabody Dec 19, 2016

Choose a reason for hiding this comment

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

Nice, does this work? I have had some trouble getting functions working when passed to the DSL.

@zachdaniel
Copy link
Author

@alanpeabody The function thing didn't work, since the DSL macro needs to unquote it. I thought it did for some reason. Good catch :D

@zachdaniel
Copy link
Author

@alanpeabody Hey guys! Any thoughts on this?

@alanpeabody
Copy link
Contributor

Hey @zachdaniel honestly is looks great, however I am hesitant to merge it until the other meta data is supported and I can see the DSL/Implementation in full. I know it probably isn't something you need but adding it isn't my top priority ATM either, though I would like to get around to it sometime.

@zachdaniel
Copy link
Author

Do you mean metadata for each included item? If so, I don't know if there is a good way to do it, because it would have to override the data that is generated by the destination serializer's metadata function. So having meta in the includes is already supported.

@asummers
Copy link
Contributor

Hi @alanpeabody Just curious if there's been any movement on this.

@suhrawardi
Copy link

Any news on this one? Would love to get it merged in as well... ! 😃

@beerlington beerlington added the needs-review Needs review from repo maintainer label Dec 14, 2018
@zachdaniel zachdaniel closed this Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-review Needs review from repo maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants