-
Notifications
You must be signed in to change notification settings - Fork 22
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
Comments
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). |
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. |
I have a general question about doing this: 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. |
I think that would be great. On a workspace based repo, the root README usually has something like the following:
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. |
I agree with @SteveCookTU. The repository's root Also, here are some of my suggestions for how to split the repo into multiple crates. I think something like this would be nice:
Have the workspace crates in a dedicated subdirectory like The The The 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. |
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. |
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. |
This is correct and it would decrease binary size when only specific ones are built |
I started the work to merge the two repositories, currently in #19 |
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 |
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 |
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.
The text was updated successfully, but these errors were encountered: