Skip to content

WIP: restructure for more modular schemes#19

Closed
msarahan wants to merge 1 commit into
timvisee:masterfrom
msarahan:modular_schemes
Closed

WIP: restructure for more modular schemes#19
msarahan wants to merge 1 commit into
timvisee:masterfrom
msarahan:modular_schemes

Conversation

@msarahan

@msarahan msarahan commented Oct 1, 2019

Copy link
Copy Markdown
Contributor

@timvisee please take this more as a tool for discussion than as actual code suggestions. It is as much a learning tool for me as it is anything else. In its current state, it does not compile.

What I've tried to do here is:

  1. Limit the public-facing API to exclusively Version and VersionManifest implementations. I'd like to get rid of VersionManifest as a means of customization, and prefer actual implementations in the "schemes" folder instead, but that's mostly out of not wanting to come up with some sort of DSL or whatever for what makes up a manifest.
  2. Use 2 traits to encapsulate the 2 kinds of functionality: parsing and ordering/comparing
  3. Define the bulk of the functionality as default implementations on traits
  4. Specialize the very limited parsing/comparing needed as the impl of the traits for the struct. This struct (Version/CondaVersion/etc.)is what makes up the public API.

I'm a bit stuck on whether there's a way to avoid the boilerplate of implementing PartialEq for each struct over and over. I want to have a default implementation that lives on the VersionCompareScheme trait. My non-working stuff is at https://github.com/timvisee/version-compare/compare/master...msarahan:modular_schemes?expand=1#diff-2bbe31eea6ca0b3d37f7a4fed78a64e1R447. I think this might be relevant discussion: rust-lang/rfcs#1024

CC @mathstuf because your discussion in #15 was a lot of the inspiration for this. I am very new to rust and very clumsy with it, so pointers would be most appreciated. Feedback/corrections would be great if you have time.

@timvisee

timvisee commented Oct 1, 2019

Copy link
Copy Markdown
Owner

Cool. Experimenting is a good idea.

Here are some thoughts on the points you gave:

  1. This sounds like a good idea. A configurable manifest like struct could
    always be implemented eventually if desired, if it would implement the scheme
    trait itself.
  2. Sounds good. I wonder if this would scale to the most common version
    notations, but that might be something to look at later.
  3. I think I'd prefer to use just the Version struct (or some different sort)
    as public API, by attaching a version scheme to it. Version comparison of
    various different kinds would go through this struct. That would allow easy
    reusability/compatability, and would hopefully prevent needing a lot of
    boilerplate.
    I think you're suggesting that Version would just act as a framework, to
    implement all kinds of version specific structs to use as public API.
    Though I think the first option would be better, the latter might be easier
    to implement if there's no proper generic way of handling this first idea.
    It's good to think about anyway!

I'm a bit stuck on whether there's a way to avoid the boilerplate of implementing PartialEq for each struct over and over. I want to have a default implementation that lives on the VersionCompareScheme trait.

Maybe the blanked-impl could work as mentioned here.
If not, a custom macro might suffice.

I have some free time available right now. I'll attempt to collect a few
different version number formats, and distill the requirements from to see what
support would be desired for.

By the way, it looks like you're doing quite well for a 'rust newbie'.
Fun fact: this crate was my first real Rust project as well from years back.

@timvisee

timvisee commented Oct 1, 2019

Copy link
Copy Markdown
Owner

I have some free time available right now. I'll attempt to collect a few
different version number formats, and distill the requirements from to see what
support would be desired for.

I did some research about versions and have written down some ideas and things I'd like.

See #20

@msarahan

msarahan commented Oct 8, 2019

Copy link
Copy Markdown
Contributor Author

Closing in favor of #21

@msarahan msarahan closed this Oct 8, 2019
@msarahan msarahan deleted the modular_schemes branch October 8, 2019 03:21
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