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

WIP: Good gc #1944

Closed
wants to merge 9 commits into from
Closed

WIP: Good gc #1944

wants to merge 9 commits into from

Conversation

Ngoguey42
Copy link
Contributor

@Ngoguey42 Ngoguey42 commented Jun 30, 2022

No description provided.

Ngoguey42 and others added 2 commits June 30, 2022 11:59
@Ngoguey42
Copy link
Contributor Author

@metanivek I have to modify your work a bit in ae2e176 in order to avoid having to make explicit casts when using functions of Io_errors.S

@codecov-commenter
Copy link

Codecov Report

Merging #1944 (b0d70ad) into main (2c6185e) will increase coverage by 0.03%.
The diff coverage is 86.00%.

@@            Coverage Diff             @@
##             main    #1944      +/-   ##
==========================================
+ Coverage   64.93%   64.96%   +0.03%     
==========================================
  Files         126      127       +1     
  Lines       15106    15125      +19     
==========================================
+ Hits         9809     9826      +17     
- Misses       5297     5299       +2     
Impacted Files Coverage Δ
src/irmin-pack/unix/errors.ml 30.00% <0.00%> (ø)
src/irmin-pack/unix/inode.ml 33.33% <ø> (ø)
src/irmin-pack/unix/io_errors.ml 45.00% <33.33%> (-0.46%) ⬇️
src/irmin-pack/layout.ml 73.33% <50.00%> (-8.49%) ⬇️
src/irmin-pack/unix/dispatcher.ml 87.50% <87.50%> (ø)
src/irmin-pack/unix/control_file_intf.ml 100.00% <100.00%> (ø)
src/irmin-pack/unix/ext.ml 74.11% <100.00%> (+0.10%) ⬆️
src/irmin-pack/unix/file_manager.ml 85.59% <100.00%> (+0.57%) ⬆️
src/irmin-pack/unix/gc.ml 83.65% <100.00%> (+0.15%) ⬆️
src/irmin-pack/unix/pack_store.ml 81.72% <100.00%> (-0.30%) ⬇️
... and 1 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@metanivek
Copy link
Member

@Ngoguey42 ah, yep. Making t abstract removed the possibility of having [> t] in function types (IIUC t and [< t] are equivalent). Getting rid of the type coercion is 💯. Thinking about this more, I wonder if we ought to eliminate Errors.Base and move all that functionality into Io_errors since we never (in practice) use just the base error manager. I don't think the separation gains much. This would leave only the error types in Errors.

@Ngoguey42
Copy link
Contributor Author

Yes, moving this code sounds like a good idea. But IMO, let's keep it as is for now and postpone that cleanup up to the day we have a second refactor needed (e.g. use that new error system in irmin-pack too)

@icristescu
Copy link
Contributor

Included in #1950

@icristescu icristescu closed this Jul 4, 2022
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.

None yet

4 participants