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.

Sign up for GitHub

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

Jump to bottom

irmin-mirage-git: update for latest versions of mirage-kv #2256

Merged
merged 3 commits into from
Jun 13, 2023
Merged

irmin-mirage-git: update for latest versions of mirage-kv #2256

merged 3 commits into from
Jun 13, 2023

Conversation

metanivek
Copy link
Member

@metanivek metanivek commented Jun 6, 2023 •
edited
Loading

This PR updates irmin-mirage-git for the latest mirage-kv (5.0.0+ 6.0.0+).

Closes #2083

@metanivek metanivek added the no-changelog-needed No changelog is needed here label Jun 7, 2023
@metanivek
irmin-mirage-git: add batch to interface
71fb78c 
It has been removed in the latest mirage-kv, but keep it here for
backwards compatibility. Doc comment copied from previous version in
mirage-kv.
@metanivek metanivek requested review from samoht, art-w and adatario June 7, 2023 13:50
@adatario
Copy link
Contributor

adatario commented Jun 9, 2023 •
edited
Loading

For reference this is (one of) the discussion that motivates the removal of batch: mirage/mirage-kv#36

adatario
adatario approved these changes Jun 9, 2023
View reviewed changes
Copy link
Contributor

@adatario adatario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Two minor stylistic nits.

src/irmin-mirage/git/irmin_mirage_git.ml Outdated Show resolved Hide resolved
src/irmin-mirage/git/irmin_mirage_git.ml Show resolved Hide resolved
@metanivek
Copy link
Member Author

For reference this is (one of) the discussion that motivates the removal of batch: mirage/mirage-kv#36

Yeah, I saw that (and Irmin-mirage-git may be closest to implementing the described semantics (I am not sure, I don't fully understand the behavior from a quick read of the code) 😅).

I do think batch should probably be re-evaluated, but didn't want to break the public API in this PR.

@codecov-commenter
Copy link

Codecov Report

Merging #2256 (d192ebe) into main (0075198) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head d192ebe differs from pull request most recent head 8b0e4fd. Consider uploading reports for the commit 8b0e4fd to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main    #2256      +/-   ##
==========================================
- Coverage   68.19%   68.17%   -0.02%     
==========================================
  Files         137      137              
  Lines       16674    16674              
==========================================
- Hits        11371    11368       -3     
- Misses       5303     5306       +3

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@metanivek metanivek requested a review from adatario June 12, 2023 20:12
adatario
adatario approved these changes Jun 13, 2023
View reviewed changes
Copy link
Contributor

@adatario adatario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@metanivek metanivek merged commit 34c2eb3 into mirage:main Jun 13, 2023
@metanivek metanivek deleted the latest-mirage-kv branch June 13, 2023 14:38
metanivek added a commit to metanivek/opam-repository that referenced this pull request Jul 4, 2023
@metanivek
[new release] ppx_irmin, libirmin, irmin, irmin-tezos, irmin-test, ir…
a31af61 
…min-pack, irmin-pack-tools, irmin-mirage, irmin-mirage-graphql, irmin-mirage-git, irmin-http, irmin-graphql, irmin-git, irmin-fs, irmin-containers, irmin-cli, irmin-chunk and irmin-bench (3.8.0)

CHANGES:

### Added

- **irmin**
  - Change behavior of `Irmin.Conf.key` to disallow duplicate key names by
    default. Add `allow_duplicate` optional argument to override. (mirage/irmin#2252,
    @metanivek)

- **irmin-pack**
  - Add maximum memory as an alternative configuration option, `lru_max_memory`,
    for setting LRU capacity. (@metanivek, mirage/irmin#2254)

### Changed

- **irmin**
  - Lower bound for `mtime` is now `2.0.0` (mirage/irmin#2166, @patricoferris)

- **irmin-mirage-git**
  - Lower bound for `mirage-kv` is now `6.0.0` (mirage/irmin#2256, @metanivek)

### Fixed

- **irmin-cli**
  - Changed `--store irf` to `--store fs` to align the CLI with what is
    published on the Irmin website (mirage/irmin#2243, @wyn)
@metanivek metanivek mentioned this pull request Jul 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adatario adatario adatario approved these changes

@samoht samoht Awaiting requested review from samoht

@art-w art-w Awaiting requested review from art-w

Assignees
No one assigned
Labels
no-changelog-needed No changelog is needed here
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

irmin-mirage-git: support mirage-kv.5.0.0
3 participants
@metanivek @adatario @codecov-commenter