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.

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

Introduce deer crate at v0.0.0 #1163

Merged
merged 40 commits into from
Oct 10, 2022
Merged

Introduce deer crate at v0.0.0 #1163

merged 40 commits into from
Oct 10, 2022

Conversation

indietyp
Copy link
Member

@indietyp indietyp commented Oct 5, 2022

🌟 What is the purpose of this PR?

deer is a new crate that implements fail-slow deserialization using multiple backends.

(currently planned: json via serde_json)

This is the first PR in many, which sets the initial project structure.

🔗 Related links

🐾 Next steps

  • Implementation of the code already in the prototype
  • derive macro
  • "branding" (preferably a logo)
  • CI & cargo-make setup Set up CI for deer #1164
  • GitHub setup (new tag?)

@github-actions github-actions bot added the area/infra Relates to version control, CI, CD or IaC (area) label Oct 5, 2022
packages/libs/deer/Cargo.toml Outdated Show resolved Hide resolved
packages/libs/deer/Cargo.toml Outdated Show resolved Hide resolved
packages/libs/deer/README.md Outdated Show resolved Hide resolved
packages/libs/deer/README.md Outdated Show resolved Hide resolved
packages/libs/deer/src/lib.rs Outdated Show resolved Hide resolved
packages/libs/deer/derive/Cargo.toml Outdated Show resolved Hide resolved
packages/libs/deer/derive/Cargo.toml Outdated Show resolved Hide resolved
packages/libs/deer/rust-toolchain.toml Show resolved Hide resolved
packages/libs/deer/rust-toolchain.toml Show resolved Hide resolved
@TimDiekmann TimDiekmann added the meta/DO NOT MERGE DO NOT MERGE. May be used in combination with `state/blocked` label Oct 5, 2022
@TimDiekmann
Copy link
Member

Very exciting 🎉

@indietyp indietyp added the area/libs > deer Affects the `deer` crate (library) label Oct 5, 2022
@indietyp indietyp mentioned this pull request Oct 5, 2022
Modifies the appendix (excluded from the unmodifiable sections 1..9) to reflect HASH-specific guidance. Reformats the LICENSE as markdown.
Reformats the license as markdown.
Someone might want to lint this; sorry!
@vilkinsons
Copy link
Member

Sorry for the direct LICENSE file commits there... thought it'd be easier than going backwards and forwards given we have a fairly rigid format.

Separately, I've added explicit reference to you in the README.md, @indietyp. We could also move this up earlier in the file, if we provide more context/narrative in the intro.

@vilkinsons vilkinsons changed the title Create new crate: deer Introduce deer crate at v0.0.0 Oct 5, 2022
@TimDiekmann TimDiekmann removed the meta/DO NOT MERGE DO NOT MERGE. May be used in combination with `state/blocked` label Oct 5, 2022
Co-authored-by: Alfred Mountfield <am@hash.ai>
@vilkinsons vilkinsons dismissed TimDiekmann’s stale review October 5, 2022 17:12

Changes requested implemented in 3de3e3d and bb95c16

@vilkinsons vilkinsons self-requested a review October 7, 2022 13:14
vilkinsons
vilkinsons previously approved these changes Oct 7, 2022
Copy link
Member

@vilkinsons vilkinsons left a comment

Choose a reason for hiding this comment

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

🎉 This is good to merge!

TimDiekmann
TimDiekmann previously approved these changes Oct 7, 2022
Copy link
Contributor

@Alfred-Mountfield Alfred-Mountfield left a comment

Choose a reason for hiding this comment

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

Thanks @indietyp, I'm really excited for this! Just requesting changes because of quite a few nits, they're not super important but I'd like us to have consistency from the get go

.github/scripts/rust/setup.py Outdated Show resolved Hide resolved
packages/libs/deer/Cargo.toml Outdated Show resolved Hide resolved
packages/libs/deer/Makefile.toml Outdated Show resolved Hide resolved
@@ -0,0 +1,113 @@
# deer
Copy link
Contributor

Choose a reason for hiding this comment

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

Purposefully lower case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, to keep in line with error-stack and how Rust crate naming usually is.

Copy link
Contributor

Choose a reason for hiding this comment

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

I appreciate that the crate names are usually lower-case, but when referring to them in headings I haven't seen them left as lower-case before.

See:

packages/libs/deer/README.md Outdated Show resolved Hide resolved
This in turn also means that deer can be used for custom validation while deserializing, while still being able to return all validation issues.

<sub>
deer might provide a way in the future to describe these constraints.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure how it'll behave inside an HTML element but should we have deer code-styled like the rest of the doc?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 14cde13

packages/libs/deer/README.md Outdated Show resolved Hide resolved
```json
{
"i8": "string"
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Would us picking something other than JSON avoid this?
image

Because I think we're going to have to deal with this on every PR for this text, not just this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed formatting for now as I wasn't able to find a way to display incomplete JSON otherwise (99538ec)

Copy link
Contributor

Choose a reason for hiding this comment

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

Dang that's quite annoying. Perhaps we can revisit later on

packages/libs/deer/macros/Cargo.toml Outdated Show resolved Hide resolved
packages/libs/deer/rust-toolchain.toml Outdated Show resolved Hide resolved
Co-authored-by: Alfred Mountfield <am@hash.ai>
Copy link
Member

@TimDiekmann TimDiekmann left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Copy link
Contributor

@Alfred-Mountfield Alfred-Mountfield left a comment

Choose a reason for hiding this comment

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

Thanks for fixing those 🎉

@indietyp indietyp merged commit 4f14a1e into main Oct 10, 2022
@indietyp indietyp deleted the bm/deer/init branch October 10, 2022 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/infra Relates to version control, CI, CD or IaC (area) area/libs > deer Affects the `deer` crate (library)
Development

Successfully merging this pull request may close these issues.

4 participants