jnioche / carbonintensity-api

A simple Rust library to retrieve data from https://api.carbonintensity.org.uk/
Apache License 2.0
10 stars 1 forks source link

Prevent start date to be before first data available #20

Closed xoen closed 1 month ago

xoen commented 1 month ago

Check that dates are not before the oldest valid date (2018-05-10 23:30:00, see issue comment)

See related Issue #15

$ cargo run -- -s 1984-08-04 bs7

Dates must be after 2018-05-10

NOTE: To avoid re-creating a new NaiveDateTime each time the validation utility function is invoked the constant is initialised lazily using Rust 1.80. However this was stabilised in Rust 1.80. This effectively means we're pushing the MSRV (minimum supported Rust version) ahead (6 releases). I think this is still better than the alternatives and the crate needs 1.74+ anyway at the moment. More information in these 2 commit messages: one, two.

xoen commented 1 month ago

On the subject of the need for 1.80+ now

I've already explained the rational in the 2 commit messages linked above but I think it's interesting to know to check these things out.

There is a rustup trick which is very useful sometimes: You can pass +toolchain to rustup-handled binaries to use a specific toolchain, examples:

$ rustc +stable --version
rustc 1.81.0 (eeb90cda1 2024-09-04)

$ cargo +nightly --version
cargo 1.83.0-nightly (ad074abe3 2024-10-04)

$ cargo +1.56 --version
cargo 1.56.0 (4ed5d137b 2021-10-04)

The toolchain would have to be installed tho, e.g. with $ rustup install 1.56.

This allows you to "go back in time" and check if something works with that version. I've done just that for a bunch of versions starting with 1.56 (Rust Edition 2021) and trying more recent versions as I was getting errors...that's how I came up with this crate old MSRV of 1.74:

$ cargo +1.56 check
    Updating crates.io index
error: failed to select a version for the requirement `chrono = "=0.4.38"`
candidate versions found which didn't match: 0.4.31, 0.4.30, 0.4.29, ...

// chrono-0.4.31 needs 1.57+: https://crates.io/crates/chrono/0.4.31

$ cargo +1.57 check
    Updating crates.io index
error: failed to select a version for the requirement `clap = "^4.4.8"`
candidate versions found which didn't match: 3.2.25, 3.2.24, 3.2.23, ...

// clap-4.4.8 needs 1.70+: https://crates.io/crates/clap/4.4.8

$ cargo +1.70 check
error: package `clap v4.5.19` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.70.0

$ cargo +1.74 check
    Checking pin-project-lite v0.2.14
    Checking core-foundation-sys v0.8.7
    [...]
    Finished dev [unoptimized + debuginfo] target(s) in 6.72s

// let's check 1.73 just in case...
$ cargo +1.73 check
error: package `clap_derive v4.5.18` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.73.0

So compiles only on Rust version >= 1.74, it's our MSRV.

PS: There are tools to automate this, etc...but I just wanted to quickly figure out which Rust versions worked and which didn't.

jnioche commented 1 month ago

PS: There are tools to automate this, etc...but I just wanted to quickly figure out which Rust versions worked and which didn't.

interesting, we should make that clear in the README that it needs >=1.74 or 1.80 as the case might be

xoen commented 1 month ago

interesting, we should make that clear in the README that it needs >=1.74 or 1.80 as the case might be

To be honest I don't wanna make too much of a fuss about the MSRV because there are all sort of things to consider about it and I just don't know enough about it :)

Bear in mind Cargo programmatically checks this courtesy of the rust-version = "1.80" I've added in the Cargo.toml - and I think that's the most important thing at this stage of this crate IMHO.

Also, AFAICT that's the field used by creates.io to display the Rust version required. Also that's the field I'd look if I wanted to check what was the MSRV of a crate.