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

fix/first release cleanup #17

Merged
merged 10 commits into from
Oct 11, 2024
Merged

fix/first release cleanup #17

merged 10 commits into from
Oct 11, 2024

Conversation

mimir-d
Copy link
Contributor

@mimir-d mimir-d commented Oct 11, 2024

  • fix docs so that they build (links, refs, formatting, etc)
  • reduce the code size for metadata inputs
    • at some cost for semantics, overload the meaning of an empty map to be
      Option::None. This however removes a bunch of boilerplate code all
      over the place
    • arguably, this can be fully avoided by not using Option in spec.rs,
      but in that case, it's clearer to match the Option with the
      non-mandatory in the spec in an 1:1 fashion
  • split runner.rs tests
  • improve api ergonomics; remove refs [api change]
    • remove ref requirement for some objects that are frequently consumed,
      without any need to keep additional objects around (or if so, they're
      Clone)
    • remove a bunch of unnecessary .clone()
  • disallow user to instantiate builders directly [api change]
    • this simplifies the api by providing a single way to make builders
  • shorten some method names [api change]
    • a bit less verbose

- fix formatting and add placeholders for missing docs
- add an action to check that docs.rs will build correctly

Signed-off-by: mimir-d <[email protected]>
- for consistency with diagnosis macros

Signed-off-by: mimir-d <[email protected]>
- at some cost for semantics, overload the meaning of an empty map to be
  Option::None. This however removes a bunch of boilerplate code all
  over the place
- arguably, this can be fully avoided by not using Option in spec.rs,
  but in that case, it's clearer to match the Option with the
  non-mandatory in the spec in an 1:1 fashion

Signed-off-by: mimir-d <[email protected]>
- this should make it easier to discover breakages before github gets to
  run the same commands; this script is relatively stable, but should be
  updated if the gh actions are changed

Signed-off-by: mimir-d <[email protected]>
- no functional changes

Signed-off-by: mimir-d <[email protected]>
- remove ref requirement for some objects that are frequently consumed,
  without any need to keep additional objects around (or if so, they're
  Clone)
- remove a bunch of unnecessary `.clone()`

Signed-off-by: mimir-d <[email protected]>
- this simplifies the api by providing a single way to make builders

Signed-off-by: mimir-d <[email protected]>
- this was missing from a publicly exported DiagnosisType, which means
  that it risks breaking crate api if changed
- for safety reasons, just make all the enums (private or public)
  non_exhaustive

Signed-off-by: mimir-d <[email protected]>
- a bit less verbose
- diagnosis was missing a verb

Signed-off-by: mimir-d <[email protected]>
@mimir-d mimir-d requested a review from njordr October 11, 2024 13:21
@mimir-d mimir-d self-assigned this Oct 11, 2024
- fixed all merge conflicts
- separate file.rs tests
- renamed new file methods to have verbs

Signed-off-by: mimir-d <[email protected]>
Copy link

codecov bot commented Oct 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.5%. Comparing base (9d59388) to head (9683219).
Report is 12 commits behind head on dev.

Additional details and impacted files

@mimir-d mimir-d merged commit a15734f into dev Oct 11, 2024
14 checks passed
@mimir-d mimir-d deleted the fix/first_release_cleanup branch October 11, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants