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

Add 6th ADR about shutting down our optparse-applicative fork #53

Merged
merged 1 commit into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions docs/ADR-6-Using-optparse-applicative-main-repository.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Status

📜 Proposed 2024-09-19

# Context

In 2021, we (the API and CLI team) wanted to improve the pretty printing of `optparse-applicative`,
so that it aligns most flags vertically. E.g. we wanted this behavior:

```
Usage: cardano-cli transaction build-raw
[ --byron-era
| --shelley-era
| --allegra-era
| --mary-era
| --alonzo-era
]
( --tx-in TX-IN
[ --tx-in-script-file FILE
[ (--tx-in-datum-file FILE | --tx-in-datum-value JSON VALUE)
( --tx-in-redeemer-file FILE
| --tx-in-redeemer-value JSON VALUE
)
--tx-in-execution-units (INT, INT)
]
]
)
[--tx-in-collateral TX-IN]
```

instead of the default - less readable - behavior:

```
Usage: cardano-cli transaction build-raw [--byron-era | --shelley-era |
--allegra-era | --mary-era |
--alonzo-era] (--tx-in TX-IN
[--tx-in-script-file FILE
[
(--tx-in-datum-file FILE |
--tx-in-datum-value JSON VALUE)
(--tx-in-redeemer-file FILE |
--tx-in-redeemer-value JSON VALUE)
--tx-in-execution-units (INT, INT)]])
[--tx-in-collateral TX-IN]
```

Sadly the [PR we proposed](https://github.com/pcapriotti/optparse-applicative/pull/428#issuecomment-1559041183) to do that was never merged by the maintainer. Which is why we did our own fork: [input-output-hk/optparse-applicative](https://github.com/input-output-hk/optparse-applicative) and depended on this fork in [cardano-cli](https://github.com/IntersectMBO/cardano-cli/blob/ca494098df110dfcc23f14ef6635ec1b062baddf/cardano-cli/cardano-cli.cabal#L245).

However, since 2021, `optparse-applicative`'s main repository [continued to evolve](https://github.com/pcapriotti/optparse-applicative/tags) and so our fork became out of date, adding to maintenance burden if we wanted to keep up.

# Decision

We want to get rid of our fork of `optparse-applicative`. Luckily, ideas from our initial PR were integrated into `optparse-applicative`'s main repo in 2023 (as mentioned [here](https://github.com/pcapriotti/optparse-applicative/pull/428#issuecomment-1559041183)), so we can now get better looking formatting of `--help` files nearly out of the box.

We want to do a PR to `optparse-applicative` with the tweak we need. This time it is going to be a way smaller change than our PR from 2021 and so we are hopeful it will be accepted.

# Consequences

We have `cardano-cli` depend on [pcapriotti/optparse-applicative](https://github.com/pcapriotti/optparse-applicative) instead of [input-output-hk/optparse-applicative](https://github.com/input-output-hk/optparse-applicative), when our PR to `optparse-applicative` is merged and released.

# References

* [Our 2021 PR](https://github.com/pcapriotti/optparse-applicative/pull/428) to `optparse-applicative`'s main repo.
* [Our fork](https://github.com/input-output-hk/optparse-applicative) of optparse-applicative.
1 change: 1 addition & 0 deletions docs/Architecture-Decision-Records.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* ✅ [[ADR-1 Default eras for CLI commands]]
* ✅ [[ADR-2 Module structure for generators]]
* 📜 [[ADR-3 Dependencies version constraints in cabal file]]
* 📜 [[ADR-6 Using optparse-applicative main repository]]

## Legend

Expand Down
Loading