search

marwanhawari / stew

🥘 An independent package manager for compiled binaries.
MIT License
193 stars 14 forks source link

:gift_heart: Batch mode for install #22

Closed cardil closed 7 months ago

cardil commented 1 year ago

Fixes #20

cardil commented 1 year ago

The PR looks to be complete @marwanhawari

I've rewritten a bunch of things to make the test run stable.

Also, to enable extracting the lib in other projects - looking forward to it.

cardil commented 1 year ago

Ping @marwanhawari

marwanhawari commented 1 year ago

Hey @cardil thanks for making a PR! I have no idea where to start with reviewing this though lol. It's 25,000+ LOC added and 100+ files changed. I know a lot of that is vendor/ code, but there are still too many changes for this 1 PR.

Could you break this up into smaller PRs? Ideally each PR should just do 1 thing. For example: 1 PR just for updating the original tests, 1 PR for refactoring error handling, etc. The smaller the better. Shoot for <250 LOC changed (not including vendor/) even if it means making 10 PRs.

Also it would be extremely helpful if you could describe in detail what you've done and why you've done it. Again, this is easier when the PRs are small.

If you can make the PRs a lot smaller, it will be much easier for me to review them and it will result in a quicker turnaround.

cardil commented 1 year ago

Hey @cardil thanks for making a PR! I have no idea where to start with reviewing this though lol. It's 25,000+ LOC added and 100+ files changed. I know a lot of that is vendor/ code, but there are still too many changes for this 1 PR.

Just, what I thought. I should have predicted this would be too much for review.

Could you break this up into smaller PRs? Ideally each PR should just do 1 thing. For example: 1 PR just for updating the original tests, 1 PR for refactoring error handling, etc. The smaller the better. Shoot for <250 LOC changed (not including vendor/) even if it means making 10 PRs.

I will. Although, that might not be so easy. I wanted to address #20, but while doing so, I identified plenty of places which could be adjusted. Let's see...

Also it would be extremely helpful if you could describe in detail what you've done and why you've done it. Again, this is easier when the PRs are small.

The main theme is described in linked issue #20, but I made some additional changes, so probably it makes sense to describe them.

If you can make the PRs a lot smaller, it will be much easier for me to review them and it will result in a quicker turnaround.

Sure, sorry for this big blob.

marwanhawari commented 7 months ago

Closing because this is too big