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

breaking change made in minor version #972

Closed
robjtede opened this issue Nov 8, 2022 · 4 comments · Fixed by #1106
Closed

breaking change made in minor version #972

robjtede opened this issue Nov 8, 2022 · 4 comments · Fixed by #1106
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@robjtede
Copy link

robjtede commented Nov 8, 2022

In v1.8.0, RunnerOptions added 3 new public fields related to retries, meaning that dependents of the hurl crate using the typical style hurl = "1" or hurl = "1.7" in their project started getting build failures today.

error[E0063]: missing fields `retry`, `retry_interval` and `retry_max_count` in initializer of `RunnerOptions`
   --> crates/hurl/src/lib.rs:268:34
    |
268 |             let runner_options = runner::RunnerOptions {
    |                                  ^^^^^^^^^^^^^^^^^^^^^ missing `retry`, `retry_interval` and `retry_max_count`

other changes to EntryResult were also breaking:

error[E0609]: no field `response` on type `EntryResult`
   --> crates/hurl/src/lib.rs:322:51
    |
322 |             if let Some(response) = &entry_result.response {
    |                                                   ^^^^^^^^ unknown field
    |
    = note: available fields are: `entry_index`, `calls`, `captures`, `asserts`, `errors` ... and 2 others

error[E0609]: no field `request` on type `&EntryResult`
   --> crates/hurl/src/lib.rs:410:47
    |
410 |                 if let Some(request) = &entry.request {
    |                                               ^^^^^^^ unknown field
    |
    = note: available fields are: `entry_index`, `calls`, `captures`, `asserts`, `errors` ... and 2 others

error[E0609]: no field `response` on type `&EntryResult`
   --> crates/hurl/src/lib.rs:423:48
    |
423 |                 if let Some(response) = &entry.response {
    |                                                ^^^^^^^^ unknown field
    |
    = note: available fields are: `entry_index`, `calls`, `captures`, `asserts`, `errors` ... and 2 others
@robjtede
Copy link
Author

robjtede commented Nov 8, 2022

Rust libraries typically avoid this conflict between a desire to add options and breaking changes by employing the builder pattern. My suggestion is that the RunnerOptions struct be deprecated externally and a RunnerOptionsBuilder struct be created to serve the same purpose until a v2 of the hurl crate is publised.

@jcamiel
Copy link
Collaborator

jcamiel commented Nov 8, 2022

Hi @robjtede thanks for the feedback.

We've been focused on the CLI Hurl binary without given a lot of attention to the public apis for our crates. As a consequence, Hurl is not well designed to be use as a library as of today. We change our public functions without thinking a lot between releases (for instance, the main function execute signature has changed between 1.6.0 and 1.7.0 , our AST model is evolving). We're going to be stricter with us in the future.

What do you advise, knowing that we're want to be able to change our code without much restriction? (maybe increment major version for each Hurl release, I see that ripgrep is at version 13 for instance)

Just by curiosity, do you have any project that use Hurl as a library that you can share?

@robjtede
Copy link
Author

robjtede commented Nov 8, 2022

Fair, I do believe there have been breaks before.

The thing is, you'll find most Cargo users are very much used to and expect crate authors to respect semver if they are usable as libraries.

What do you advise, knowing that we're want to be able to change our code without much restriction?

Splitting the crate up and having a libhurl crate (so that versions can be independently updated) that is used by hurl (the binary) offers a few advantages:

  • binary-only crates are much more tolerant to technically-breaking changes in minor version bumps
  • libhurl can start again from version 0.1 and be incremented with breaking changes to 0.2, 0.3, etc, much faster while still respecting the flavor of semver that the Rust ecosystem uses
  • you'll become subconciously thoughtful about the libhurl API which will improve semver compliance and, i'd expect, overall code quality too sorry, bit preach-y, and the crate already has a lib/bin distinction, derp

You also could just increment the major versions like ripgrep does; I don't think there's anything particularly wrong with that approach.

Just by curiosity, do you have any project that use Hurl as a library that you can share?

Sadly no, we've integrated it into an internal tool at work that can run all sorts of useful routines for our specific system during development. At a high level, it's a pretty dumb passthrough with some pre-set context values, some templating possibilities using Tera (so they tell me), glob file selection, and nicer printed output formatting for multi-test file runs.

@jcamiel
Copy link
Collaborator

jcamiel commented Nov 29, 2022

Hi @robjtede just for your information, we've added a check in our CI to detect major breaking changes. Actually, the next version of Hurl will be a new major one (2.0.0). I will try to look into the builder pattern as suggested to minimise change when we add options.

@jcamiel jcamiel self-assigned this Dec 10, 2022
@jcamiel jcamiel linked a pull request Dec 16, 2022 that will close this issue
@jcamiel jcamiel added this to the 2.0.0 milestone Dec 18, 2022
@jcamiel jcamiel added the enhancement New feature or request label Dec 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants