Skip to content
This repository has been archived by the owner on Jan 27, 2022. It is now read-only.

feat(Claim): Claim type #255

Merged
merged 6 commits into from
Apr 13, 2021
Merged

feat(Claim): Claim type #255

merged 6 commits into from
Apr 13, 2021

Conversation

rgieseke
Copy link
Collaborator

@rgieseke rgieseke commented Apr 7, 2021

A very first attempt at a Claim spec.
See discussion in stencila/encoda#885

Not quite sure about title and claimType. Should the kind be pre-specified, not sure ...

@codecov-io
Copy link

codecov-io commented Apr 7, 2021

Codecov Report

Merging #255 (eb7ed70) into master (379cf19) will not change coverage.
The diff coverage is n/a.

❗ Current head eb7ed70 differs from pull request most recent head 1d08d57. Consider uploading reports for the commit 1d08d57 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #255   +/-   ##
=======================================
  Coverage   96.28%   96.28%           
=======================================
  Files          10       10           
  Lines         296      296           
  Branches       72       72           
=======================================
  Hits          285      285           
  Misses         11       11           
Flag Coverage Δ
py 98.03% <ø> (ø)
ts 95.91% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 379cf19...1d08d57. Read the comment docs.

Copy link
Member

@nokome nokome left a comment

Choose a reason for hiding this comment

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

Thank you @rgieseke 🙏🏽 . Loving these PR's keep 'em coming! 🙂

Looks good, I have made some suggestions for further improvements. Also, could you please add Claim to BlockContent.schema.yaml.

schema/Claim.schema.yaml Outdated Show resolved Hide resolved
schema/Claim.schema.yaml Outdated Show resolved Hide resolved
schema/Claim.schema.yaml Outdated Show resolved Hide resolved
schema/Claim.schema.yaml Outdated Show resolved Hide resolved
schema/Claim.schema.yaml Outdated Show resolved Hide resolved
schema/Claim.schema.yaml Show resolved Hide resolved
@rgieseke
Copy link
Collaborator Author

rgieseke commented Apr 7, 2021

Thank you for prompt feedback and guidance! 🙇‍♂️
Do you have a preference how commit messages (with GitHub suggestions i don't get the gentle nudges)?
Should they be rebased before a merge?

@nokome
Copy link
Member

nokome commented Apr 7, 2021

Do you have a preference how commit messages (with GitHub suggestions i don't get the gentle nudges)?

Thanks for asking. I've added a guide here and installed this on the repo for nudges. No need to change your existing commit messages.

Should they be rebased before a merge?

No need, but feel free to if it makes merge conflicts go away for example

@rgieseke
Copy link
Collaborator Author

@nokome nokome changed the title feat(Claim): Draft Claim specification feat(Claim): Claim type Apr 13, 2021
@nokome
Copy link
Member

nokome commented Apr 13, 2021

Not sure what went wrong in the pipeline, look like a TypeScript deployment problem

The problem was that we were attempting to release docs to GitHub pages on an external PR using an internal token, which Azure Pipelines rightly refuses access to. I've added a condition to only release TS docs on tags (as for other langs).

I also noticed that @id should be schema:Claim since this is an existing schema.org type and that label was incorrectly indented. Lastly, I've ordered the properties alphabetically as it's our loose convention.

@nokome nokome merged commit 1d08d57 into stencila:master Apr 13, 2021
@stencila-ci
Copy link
Collaborator

🎉 This PR is included in version 1.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@rgieseke
Copy link
Collaborator Author

I also noticed that @id should be schema:Claim since this is an existing schema.org type and that label was incorrectly indented. Lastly, I've ordered the properties alphabetically as it's our loose convention.

Good catch and i learnt another thing, thanks! :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants