-
Notifications
You must be signed in to change notification settings - Fork 123
Promote "Metadata in Table Schema" recipe to the specs. #961
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
Conversation
|
Thanks a lot for rebasing the PR @pierrecamilleri ! |
|
Almost there. I added "partial" to qualify "Data Resource" in the suggestion of Peter concerning the documentation of top-level "examples" ; because an object with |
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.
@pierrecamilleri thanks for the updates! Regarding:
Or do we want to impose a name property ? What do you think of this wording ?
I think we should simply stick to the Data Resource spec and require a name (over a title). I have made suggestions to the files to reflect this change.
Co-authored-by: Peter Desmet <peter.desmet.work@gmail.com>
Co-authored-by: Peter Desmet <peter.desmet.work@gmail.com>
|
Hi all, we added you as a reviewer as you are a voting member and this PR is tagged "Requires 6 voting approvals" ! We would like this PR to land soon, if you can find the time to make a review it would be great ! |
PietrH
left a comment
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.
Looks good, I made some small textual suggestions.
khughitt
left a comment
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.
Looks good to me. 👍
ezwelty
left a comment
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.
End-of-year crunch so admittedly only a cursory review, but looks good to me.
pschumm
left a comment
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.
This looks ok to me.
Co-authored-by: Pieter Huybrechts <48065851+PietrH@users.noreply.github.com>
Apply review suggestions
|
Thanks a lot @pierrecamilleri! I already voted |
|
|
|
Great work @pierrecamilleri 👏 Looks like it's ready to merge now 🚀 |
|
Hi @pierrecamilleri, Sorry for the slow communication due to the holiday season =) Looks great! We just need to remove any changes to |
Done! (Sorry I have missed this) |
|
Amazing! Thanks! |
|
Merged. Here is a live preview of |
Context and related issue :
fix #899
Migrated pull request from datapackage-v2-draft repo
Still to do on this PR: