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

Split codebase into a workspace of multiple library crates #46

Merged
merged 4 commits into from
Jul 3, 2020

Conversation

Hirevo
Copy link
Owner

@Hirevo Hirevo commented Apr 12, 2020

This PR factors out some modules of the alexandrie binary crate into their own library crates and transforms the project into a workspace in the process.

Here is a description of what the different crates are:

  • alexandrie:
    No change here, this is the main binary crate just as before, but it now makes use of the other libraries in the workspace.
  • alexandrie-index:
    This crate contains the definitions of the Indexer trait and all of its implementors (currently CommandLineIndex and Git2Index).
    It also contains the relevant configuration structs for all of these types.
  • alexandrie-storage:
    This crate contains the definitions of the Store trait and all of its implementors (currently only DiskStorage).
    Similarly, it also contains the relevant configuration structs for these types.
  • alexandrie-rendering:
    This crate contains everything needed to perform Markdown rendering (including the syntax-highlighting of the code blocks).
    It also contains all the syntect-related configuration bits (like reading dumps or traversing directory trees to load syntax and theme definitions).

We'll most-likely split the binary crate even more, in the future, to separate even more concerns and/or expose some more of the registry's mechanisms to any developer.

The motivations for this is:

  • To improve maintainability:
    Having multiple crates in the project splits concerns, even at the dependency management level, so each library have its own dependencies that the main binary crate doesn't need to have.
    And Cargo makes managing these multiple crates super easy, all the previous commands like cargo build or cargo update just works across the workspace.
  • To improve testability:
    With separate crates, we can better write integration tests for each part of the system (the index management part, the crate storage part, the README rendering part, etc...).
  • To allow reuse of previously internal components:
    With this change, other codebases have access to the same types that the registry uses, therefore, we can, for instance, more easily build companion tools, like a health-checking tool for the registry (a binary that could check that the crate index, the database and the crate storage are all in-sync) or the README rendering tool we're currently working on (Companion tool to manage renderings of READMEs #45).

@Hirevo Hirevo added the C-refactor Category: Refactor label Apr 12, 2020
@Hirevo Hirevo self-assigned this Apr 12, 2020
@danieleades
Copy link
Contributor

while you're looking at the 'librification' of Alexandrie, you might like to consider adopting Crate-Index (or at least some of its types). It's Tree object is maybe a little more mature. (The git side is going to remain immature until i'm comfortable i can write integration tests for authentication.)
It's almost 100% unit-tested and documented, though it may not yet have all the methods you need. the Tree struct cannot implement your Indexer trait without interior mutability (for performance reasons, it caches some information, which requires &mut self).

A little about my motivation here

  • i'm desperate to support adoption of Rust in the company i work in
  • a lack of a self-hosted crate registry is a blocker
  • a rock-solid, correct, and well-tested implementation is a MUST
  • a GUI is nice to have, but not required. similarly for tools to filter and search.
  • integration with Artifactory (for binary storage) is probably a requirement

To meet those goals, i'm trying to both

  • create a rock-solid, and well-tested index management crate as one piece of a future web API. I will then create a web API as a thin wrapper around that. (for me modularity and a user-interface are non-goals)
  • contribute to Alexandrie wherever I can (particularly the backend) to assist in making it production ready

To be honest i don't really mind which works out first, or if they meet in the middle!

@Hirevo
Copy link
Owner Author

Hirevo commented Apr 28, 2020

Adopting Crate-Index (even partially) is definitely a possibility as the APIs and functionality of some of the types there are more complete than what we have here (like we don't do validation or canonicalization of crate names, even though we really should by now, or things like an index builder), I find it very well done.
I also like the fact that it is asynchronous, though, if Alexandrie were to use it right now, we would have to use the blocking module.
Alexandrie should ideally be fully asynchronous but a problem with Diesel makes this complicated (its database transaction API kinda prevents the use of asynchronous functions, I hope, one day, to come up with a solution to this).

About the Indexer problem, I agree it should be revised.
I opened #60 to discuss exactly how to address its shortcomings.

@Hirevo Hirevo force-pushed the refactor/workspace branch from 501e8b4 to 0b4cc36 Compare July 2, 2020 23:37
@Hirevo Hirevo marked this pull request as ready for review July 3, 2020 17:22
@Hirevo
Copy link
Owner Author

Hirevo commented Jul 3, 2020

I think we can move forward with this initial library split, keeping it in a PR would require doing rebases each time we make a change on master.
The CI confirms that we (allegedly) did not break anything with this.
I think we can continue to split even more in subsequent PRs instead of trying to make a very big one to always keep up with master.

@Hirevo Hirevo merged commit ca2c787 into master Jul 3, 2020
@Hirevo Hirevo deleted the refactor/workspace branch July 3, 2020 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-refactor Category: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants