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.

Sign up for GitHub

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

Jump to bottom

docs: 📝 using jsonschema tool for checking metadata #134

Merged
merged 7 commits into from
Nov 13, 2024
Merged

docs: 📝 using jsonschema tool for checking metadata #134

merged 7 commits into from
Nov 13, 2024

Conversation

martonvago
Copy link
Contributor

Description

This adds most of a decision post on a new tool for checking metadata against the Data Package standard.

I've left the conclusion empty for now, as I'd like to hear what everyone thinks!

Closes #131

Reviewer Focus

This PR needs an in-depth review.

Focus on what decision to make.

Checklist

  • Ran spell-check
  • Formatted Markdown
  • Rendered website locally

@martonvago martonvago self-assigned this Nov 6, 2024
@martonvago martonvago changed the title docs: 📝 add pros and cons of tools for checking metadata docs: 📝 new tool for checking metadata Nov 6, 2024
@martonvago martonvago marked this pull request as ready for review November 6, 2024 15:44
@martonvago martonvago requested a review from a team as a code owner November 6, 2024 15:44
martonvago
martonvago commented Nov 7, 2024
View reviewed changes
why-choice/index.qmd Outdated Show resolved Hide resolved
lwjohnst86
lwjohnst86 requested changes Nov 7, 2024
View reviewed changes
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Very nice! Some small comments

why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
@martonvago @lwjohnst86
apply suggestions from code review
ac9d60b 
Co-authored-by: Luke W. Johnston <[email protected]>
K-Beicher
K-Beicher previously approved these changes Nov 8, 2024
View reviewed changes
Copy link
Contributor

@K-Beicher K-Beicher left a 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

signekb
signekb requested changes Nov 11, 2024
View reviewed changes
Copy link
Member

@signekb signekb left a comment

Choose a reason for hiding this comment

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

Nice work, Marton 🌟
Some of my suggestions might be a matter of preference, so if you disagree, you can just resolve them :)

why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
why-choice/index.qmd Outdated Show resolved Hide resolved
@martonvago @signekb
apply suggestions from code review
a98bde2 
Co-authored-by: Signe Kirk Brødbæk <[email protected]>
signekb
signekb previously approved these changes Nov 11, 2024
View reviewed changes
Copy link
Member

@signekb signekb left a comment

Choose a reason for hiding this comment

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

Great ⭐

K-Beicher
K-Beicher previously approved these changes Nov 12, 2024
View reviewed changes
Copy link
Contributor

@K-Beicher K-Beicher left a comment

Choose a reason for hiding this comment

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

Nicely done, very easy to read, only one small correction and a suggestion that is much more taste/style than anything else.

why-jsonschema/index.qmd Show resolved Hide resolved
why-jsonschema/index.qmd Show resolved Hide resolved
@lwjohnst86 lwjohnst86 changed the title docs: 📝 new tool for checking metadata docs: 📝 using jsonschema tool for checking metadata Nov 13, 2024
lwjohnst86
lwjohnst86 requested changes Nov 13, 2024
View reviewed changes
Copy link
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

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

Looks great! Only a small edit to add a link to our decision post, then I will merge 👍

why-jsonschema/index.qmd Outdated Show resolved Hide resolved
@lwjohnst86 lwjohnst86 merged commit 576cfc0 into main Nov 13, 2024
@lwjohnst86 lwjohnst86 deleted the docs/why-tool-for-checking-metadata branch November 13, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@signekb signekb signekb left review comments

@lwjohnst86 lwjohnst86 lwjohnst86 approved these changes

@K-Beicher K-Beicher K-Beicher left review comments

Assignees

@martonvago martonvago

Labels
None yet
Projects
Team project planning
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Decision post for checking metadata/properties
4 participants
@martonvago @lwjohnst86 @signekb @K-Beicher