-
Notifications
You must be signed in to change notification settings - Fork 120
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
Promote "Metadata in Table Schema" recipe to the specs. #961
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 <[email protected]>
Co-authored-by: Peter Desmet <[email protected]>
I merged the |
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 ! |
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.
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. 👍
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.
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 <[email protected]>
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: