Add `MaxReg` and `MinReg` registers #146

DaveLanday · 2024-04-24T20:07:44Z

This PR comes from the following discussion: #145 (comment). MaxReg and MinReg CRDTs track a monotonically increasing or decreasing value respectively. The struct has a single field called val to maintain consistent format with the LWWReg CRDT, which has fields val and marker. MaxReg and MinReg do not need a marker as they are monotonic values and can technically be considered a marker. The single field val is preferred to pub struct<V: Ord Copy> MinReg(V) as it is clear what the struct contains and will allow adding any other fields in the future. A field that might be interesting to add could be bound which acts as an upper/lower bound on the opaque value stored val. This could be used as a convenient termination signal or as a way to test whether distributed replicas of a MinReg or MaxReg have reached some agreed upon state etc...

…r decreasing value respectively. The struct has a single field val to maintain consistent format with the rest of the library. Possibly we want to add another field that can represent a bound (upper, lower) on the size of val.

src/maxreg.rs

src/minreg.rs

davidrusu

Generally looks good, just mainly some style points to ensure the library has a uniform feel across CRDT's in the same family.

DaveLanday · 2024-05-02T15:29:16Z

Ok! I will resolve these. Thanks @davidrusu

…for read and write functions. Modified test_update for MinReg and MaxReg

DaveLanday · 2024-05-02T20:44:18Z

Hi @davidrusu I just pushed the changes. Let me know if there are any other steps to take / changes to make.

src/maxreg.rs

src/minreg.rs

davidrusu

just a few more nits

…ead of self.merge. Applies suggestions in PR rust-crdt#146

DaveLanday · 2024-05-03T17:03:31Z

just a few more nits

Hi @davidrusu, no problem at all. Hopefully I followed all of your suggestions correctly at this point, but happy to fix anything else you think isn't conforming to the format.

davidrusu

excellent, thank you

davidrusu · 2024-05-04T15:13:22Z

@DaveLanday looks like the doc tests are failing, can you fix those? You can run doc tests with

cargo test --doc

Also, while your at, make sure that cargo clippy is clean and run the auto formatter: cargo fmt

…ing semi-colons, and struct fields.

DaveLanday · 2024-05-06T13:37:20Z

Hi @davidrusu I fixed the issues (typos, etc...) It passes cargo test --doc now! cargo clippy is also clean, and the formatter ran no problem! Cheers

…and write functions. Modified test_update for MinReg and MaxReg

…ead of self.merge. Applies suggestions in PR #146

davidrusu · 2024-05-06T15:45:06Z

Thanks!

…ate instead of self.merge. Applies suggestions in PR #146" This reverts commit cbd481a.

…or read and write functions. Modified test_update for MinReg and MaxReg" This reverts commit 9ceb1e2.

davidrusu · 2024-05-06T15:52:38Z

@DaveLanday heads up, you should set your email in your git config, as it stands, the commits you had made are not linked to your profile! I'm mentioning so that you get credit for your work in the future!

DaveLanday · 2024-05-06T17:22:45Z

@DaveLanday heads up, you should set your email in your git config, as it stands, the commits you had made are not linked to your profile! I'm mentioning so that you get credit for your work in the future!

Ooooo! thank you for letting me know. Is there anyway to get recognition after git config --global [email protected] ?

davidrusu · 2024-05-08T06:19:16Z

The commits are still linked to this PR so people can find you that way, but they just don't link directly to your profile the way that other commits do. There's nothing more to do now for these commits, but future commits will be linked correctly if you commit using an email that is used in your github profile

davidrusu reviewed Apr 29, 2024

View reviewed changes