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

Convert Repo to Workspace #7

Open
SteveCookTU opened this issue Feb 14, 2023 · 11 comments
Open

Convert Repo to Workspace #7

SteveCookTU opened this issue Feb 14, 2023 · 11 comments

Comments

@SteveCookTU
Copy link
Contributor

I was thinking it would be useful to turn the base directory of the repository into a workspace. This way other crates this lib may depend on, such as the derive macro, can also be modified within the same PR when necessary.

I bring this up because I was looking into replacing the split type and values for decoding and encoding hints (Rust enums can take care of these as one enum). To make those changes, the derive macro would also have to be changed which I would need to open a PR for on the other repo along with needing to wait for it to be published.

@hschimke
Copy link
Collaborator

You have hit on something I generally agree with, I think I would like to do that, and thinking of the derive macros, I haven’t moved them into the organization yet, so that would make it even more confusing.

The encode / decode HashMaps are certainly not my favorite. I went with doing it the “Java” way while porting because so much of the code was built around that, but now that the port is done it makes sense to change (probably to a HashSet of just DecodeHintValues, but they could then become DecodeHints because the Values/Types separation would evaporate, etc).

I went from just barely knowing rust when I started this project to having a pretty firm grasp on it at the end, so some of my decisions early on turned out to be mistakes (some of them organizational, some of them technical).

@hschimke
Copy link
Collaborator

I’ve thought about this more, I’m going to go ahead and do this change, though the timeline is at least two weeks while I finish up some other stuff.

@hschimke
Copy link
Collaborator

I have a general question about doing this:
How should I lay out the README files for this workspace projects?

My primary thoughts are: The main entry point for the project is the root, and the part people actually care about is the root. No one is all that interested in the macros but maintainers / developers.

I think I’m going to basically keep the README as is, but add a section describing what the macro crate is, and the structure of the repository. Then I’d add more detailed information in the README for the library itself (possibly more usage or code conventions, better API documentation, etc). Then for the macro folder I would (well, I would say expand, but really it’s rewriting entirely because there is no documentation in the macro crate). Probably that needs to become a better description of exactly what is happening in the different macros and a specific explanation of when and where we’re using them.

This is all just me thinking out loud, but feel free to chime in if you have thoughts or know a better practice.

@SteveCookTU
Copy link
Contributor Author

I think that would be great. On a workspace based repo, the root README usually has something like the following:

  • Crates.io, docs, CI build, links through badges
  • A general "about" that describes the library as a whole
  • Small usage snippet(s)
  • Explanation of features
  • "Learn More" section that has links to more in-depth documentation and examples, and sometimes a book link if a lib provides one
  • Roadmap or current status

Then the crates within the workspaces have their own that describe what they are to the library. For the core of this crate, it probably doesn't need an extra README. The macro one could just describe the macros it provides or something.

If anything I'd recommend seeing the layout other popular libs use and customize it to your liking or remove things you might deem unnecessary to the usage of the crate.

@Asha20
Copy link
Contributor

Asha20 commented Feb 21, 2023

I agree with @SteveCookTU. The repository's root README.md file is basically like a homepage. It's the first thing someone sees when they open your repo. As such, it should explain what the code aims to achieve. The "how it works under the hood" isn't as important to someone just trying to get an idea of whether they can use your library or not. You can relegate this info to some other file, and maybe link to it from the root README.md (somewhere towards the bottom).

Also, here are some of my suggestions for how to split the repo into multiple crates. I think something like this would be nice:

. 
└─ crates 
   ├─ common 
   ├─ core 
   ├─ cli
   ├─ [proc macro crates]
   └─ [one crate for each logical encoder/decoder unit] 

Have the workspace crates in a dedicated subdirectory like crates instead of in the repo root to make things less messy when you have a large number of crates.

The common crate should hold things that are used by multiple encoders/decoders. For example, Point is a prime candidate for this crate. If some logic is used in only one place, it should go in that particular encoder/decoder's crate, not in common. If you see that common is getting too large though, it might be worth extracting multiple crates from it as well.

The core crate brings everything together. You can think of it as an entry-point to your library. It pulls the other workspace crates and exports what's needed.

The cli crate holds the CLI. It has a dependency of core.

After that, have one crate per logical encoder/decoder unit. I'm not too familiar with the various barcodes, but I'm guessing that the logic for Code 39, Code 93 and Code 128 barcodes is pretty similar. If so, put them in a single crate together so that they can re-use common logic easily.

@SteveCookTU
Copy link
Contributor Author

I never even thought about extracting the readers into their own crates and I think that would be good. Doesn't have to be at first but might be good in the future.

Now that I think about it, every major package in Java could represent its own crate. I think this could make things more modular and would help organization.

@hschimke
Copy link
Collaborator

Interesting, I had never considered breaking it up past the reader/writer level before, and I hadn’t considered moving the cli into the main repository, though it makes sense. I feel like breaking up symbologies might help move towards something I’ve been wanting to do for a while, which is allow much more modular builds of the library. Currently, outside turning off a few very specific features, every symbology is always built. I can certainly imagine a scenario where someone would prefer to only build in one-d symbologies, or perhaps only datamatrix and qrcode.

Other sources lead me to believe that this might speed up build time as well, but I don’t know enough about that to really be certain.

Your intuition was correct: all the one-d libraries are very similar. It’s why they are the only place where I ended up using derive macros. Some of that might be my lack of imagination when porting out of the inheritance structure that they had in Java.

@SteveCookTU
Copy link
Contributor Author

SteveCookTU commented Feb 21, 2023

Other sources lead me to believe that this might speed up build time as well, but I don’t know enough about that to really be certain.

This is correct and it would decrease binary size when only specific ones are built

@hschimke
Copy link
Collaborator

I started the work to merge the two repositories, currently in #19

@hschimke
Copy link
Collaborator

hschimke commented Mar 1, 2023

And now the CLI too. I think I'm going to merge #19 to make this work accessible so we can start using it.

I'm going to keep that branch alive, though, and start working on fully reorganizing the repository.

There is a security alert for a version of the time crate, so after the merge passes, I'm going to bump the version on all the dependencies and publish new versions on crates.io

@hschimke
Copy link
Collaborator

hschimke commented Mar 1, 2023

I have a working branch where common has been broken out into a new crate. I haven't gone too far with it yet, and the work wasn't difficult enough that I wouldn't consider just throwing it away if I did something wrong. I think it's a solid proof of concept that it's not going to be impossible to move out of the current monolithic crate.

https://github.com/rxing-core/rxing/tree/re-organize-repository

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

No branches or pull requests

3 participants