-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
[PROPOSAL] Add Download Attribute Transformer #1710
Conversation
@@ -509,6 +509,27 @@ downloading from the `download` URL. It is recommended that this field is | |||
only generated by automated tools (where it is encouraged), | |||
and not filled in by hand. | |||
|
|||
##### download_hash | |||
|
|||
If supplied, `download_hash` is an array of hash digests. Currently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"array" -> "object"
LGTM |
@dbent good spot, I missed updating that. |
@KSP-CKAN/wranglers @dbent I've done a bunch of extra testing on some random netkans and it looks to be doing what is intended.
|
This is good to merge, whenever you're ready. |
Neat, I'll let you do the honors if you like @dbent - I'll run the indexer manually and squash all the commits before pushing them. |
MERGE PARTY 🍰 🎈 🎉 |
👯 |
@dbent Based on your feedback and some thoughts I had this morning. Extending on #1703 and addressing extra requirements in #1682
NOTE: @KSP-CKAN/wranglers if this gets merged, the next index run will make a lot of changes.
I've merged DownloadHashTransformer + DownloadSizeTransformer into DownloadAttributeTransformer, along with adding 'download_content_type'. Seemed to make sense to have all those three together. Tests + Spec have also been added/updated.
Following Netkan
Produces the following ckan
Which passes validation