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

irmin-cli: change --store irf to --store fs #2243

Merged
merged 2 commits into from
May 19, 2023

Conversation

wyn
Copy link
Contributor

@wyn wyn commented May 2, 2023

The information in the Irmin website uses "fs" where-as the irmin-cli Resolver uses "irf". Changed command line and module sig to use fs and also updated cram tests.

NOTE previous use of "irf" will now error, I wasn't sure whether backwards compatibility was expected?

As an example, looking at the cli --help segment:

before

       -s VAL, --store=VAL
           The storage backend (one of git, git-mem, irf, mem, mem-http,
           git-http, pack or tezos). Default is `git'.

after

       -s VAL, --store=VAL
           The storage backend (one of git, git-mem, fs, mem, mem-http,
           git-http, pack or tezos). Default is `git'.

@metanivek metanivek linked an issue May 2, 2023 that may be closed by this pull request
Copy link
Member

@metanivek metanivek left a comment

Choose a reason for hiding this comment

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

This looks great. Backwards compatibility is fine to break for this usage, especially since it was wrong wrt the docs. 🙈

Can you also put an entry in CHANGES.md mentioning the fix? (just insert a new ## Unreleased section at the top and follow the pattern of previous entries)

Thanks!

@wyn
Copy link
Contributor Author

wyn commented May 2, 2023

This looks great. Backwards compatibility is fine to break for this usage, especially since it was wrong wrt the docs. 🙈

Can you also put an entry in CHANGES.md mentioning the fix? (just insert a new ## Unreleased section at the top and follow the pattern of previous entries)

Thanks!

no probs, just force pushed that too.

Some context, I'm here on the mirageOS retreat and trying to learn more about Irmin so I thought I'd try some beginner bugs.

@codecov-commenter
Copy link

Codecov Report

Merging #2243 (fc0b5be) into main (4c38a4b) will decrease coverage by 0.02%.
The diff coverage is 16.66%.

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

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #2243      +/-   ##
==========================================
- Coverage   67.93%   67.91%   -0.02%     
==========================================
  Files         136      136              
  Lines       16603    16605       +2     
==========================================
- Hits        11279    11278       -1     
- Misses       5324     5327       +3     
Impacted Files Coverage Δ
src/irmin-pack/unix/gc.ml 69.79% <0.00%> (-3.62%) ⬇️
src/irmin-cli/resolver.ml 53.64% <50.00%> (ø)

... and 1 file with indirect coverage changes

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

@wyn wyn force-pushed the swp-2234-irmin-cli-change-irf-to-fs branch from 6114435 to ff16de9 Compare May 2, 2023 15:24
@metanivek
Copy link
Member

Some context, I'm here on the mirageOS retreat and trying to learn more about Irmin so I thought I'd try some beginner bugs.

Awesome -- enjoy the retreat! 🐫

@wyn wyn force-pushed the swp-2234-irmin-cli-change-irf-to-fs branch from ff16de9 to 2670175 Compare May 16, 2023 09:32
CHANGES.md Show resolved Hide resolved
The information in the Irmin website uses "fs" where-as
the irmin-cli Resolver uses "irf". Changed command line
and module sig to use fs and also updated cram tests.

NOTE previous use of "irf" will now error.
@wyn wyn force-pushed the swp-2234-irmin-cli-change-irf-to-fs branch from 2670175 to 2e10154 Compare May 19, 2023 09:27
@metanivek metanivek merged commit c4e92ac into mirage:main May 19, 2023
@metanivek
Copy link
Member

Thanks!

metanivek added a commit to metanivek/opam-repository that referenced this pull request Jul 4, 2023
…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)
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.

irmin-cli: change --store irf to --store fs
3 participants