Conversation
6d0fe23 to
b884036
Compare
3922850 to
238d9d5
Compare
76c1521 to
4af3ab9
Compare
palas
left a comment
There was a problem hiding this comment.
Here are some comments of what I think are issues. Overall, it looks very complete. I think I need to try it a bit too, next week. I am surprised about some implementations from scratch for things like the interactive mode and the parsing of yaml, but not sure if they are a good or a bad idea. For example, the checking of '\r' and '\n' independently I suspect may give trouble for windows, because it produces both.
| where | ||
| renderKind (name, kd) = | ||
| [" " <> name <> ":"] | ||
| <> [" notable: false" | not $ kindNotable kd] |
There was a problem hiding this comment.
Since it is being generated automatically, it would probably be better to make notable: true explicit, since it is more self-documenting
There was a problem hiding this comment.
I was thinking about that. It was adding noise to the generated configuration file IMO. I've added a comment saying that it is default. If you'd like still explicit notable: true I can that.
, "# Each kind defines:"
, "# notable: if true (default), changes of this kind appear in the changelog"
, "# if false, they still bump the version but are omitted from the changelog text"
, "# bump: PVP version component to bump when this kind is present"
, "# 0.1.0.0 = bump 2nd component (breaking changes)"
, "# 0.0.1.0 = bump 3rd component (features, compatible changes)"
, "# 0.0.0.1 = bump 4th component (patches, internal changes)"
, "#"
There was a problem hiding this comment.
If it makes it harder to read maybe it is better to not write it, it is a good point. I don't have a strong opinion
| instance ToJSON KindDef where | ||
| toJSON kd = | ||
| object | ||
| $ ["notable" .= kindNotable kd | not $ kindNotable kd] |
There was a problem hiding this comment.
Pull request overview
This PR introduces herald, a Haskell-based release aid tool that manages checked-in changelog fragments under .changes/, validates them in CI, and automates per-package release batching/version bumps (including reusable GitHub Actions and Nix flake packaging).
Changes:
- Add the
heraldCLI (init/new/validate/batch/next) with fragment parsing/rendering, PVP bump logic, git helpers, and changelog/.cabal updates. - Add unit + e2e test suites (Hedgehog/Tasty) plus shared test utilities/fixtures.
- Integrate herald into Nix flake outputs and ship reusable GitHub Actions + example workflows for validation and releases.
Reviewed changes
Copilot reviewed 46 out of 48 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| herald/test/Test/Herald/Terminal.hs | Unit tests for ANSI stripping behavior used by terminal input. |
| herald/test/Test/Herald/Render.hs | Unit tests for changelog section rendering and sorting/filtering rules. |
| herald/test/Test/Herald/Pvp.hs | Unit + property tests for PVP parsing/showing and bumping. |
| herald/test/Test/Herald/Git.hs | Tests for repo slug parsing and git config lookup behavior. |
| herald/test/Test/Herald/Fragment.hs | Tests for fragment YAML decoding and validation rules. |
| herald/test/Test/Herald/Config.hs | Tests for config/kind parsing + serialization behaviors. |
| herald/test/Test/Herald/Cabal.hs | Tests for reading/writing versions in .cabal files. |
| herald/test/Main.hs | Unit test suite runner wiring. |
| herald/test-lib/Test/Herald/Fixtures.hs | Shared test fixtures/configs and PVP helpers. |
| herald/test-lib/Test/Herald/Assertions.hs | Shared Hedgehog assertion helpers for better failure messages. |
| herald/test-e2e/Test/Herald/E2E/Validate.hs | E2E coverage for validateFragment/files/diff/pr flows with git fixtures. |
| herald/test-e2e/Test/Herald/E2E/Next.hs | E2E tests for computing next version from fragments + cabal version. |
| herald/test-e2e/Test/Herald/E2E/Init.hs | E2E tests for repo scanning and config/template generation. |
| herald/test-e2e/Test/Herald/E2E/Fragment.hs | E2E tests for herald new fragment creation semantics. |
| herald/test-e2e/Test/Herald/E2E/Fixtures.hs | E2E git repo setup helpers + test configs for diff scenarios. |
| herald/test-e2e/Test/Herald/E2E/Batch.hs | E2E tests for batching, changelog updates, commits/tags, idempotency. |
| herald/test-e2e/Main.hs | E2E test suite runner wiring. |
| herald/src/Herald/Types.hs | Core types + JSON instances; default kinds; user-facing exception type. |
| herald/src/Herald/Terminal.hs | Interactive prompting utilities + ANSI stripping. |
| herald/src/Herald/Pvp.hs | PVP parsing/showing and bump implementation. |
| herald/src/Herald/Git/Repository.hs | Filesystem-based .git reading (HEAD/tags/config). |
| herald/src/Herald/Git.hs | Higher-level git operations + diff/changed-files via git process. |
| herald/src/Herald/Fragment/Render.hs | Markdown rendering for changelog sections and notable filtering. |
| herald/src/Herald/Fragment/Read.hs | Fragment discovery and loading from .changes/. |
| herald/src/Herald/Fragment.hs | Fragment validation against configured projects/kinds. |
| herald/src/Herald/Config.hs | YAML config loader producing parse errors as Either. |
| herald/src/Herald/Command/Validate.hs | Implements validate subcommand logic for files/diff/pr checks. |
| herald/src/Herald/Command/Next.hs | Implements next subcommand to compute next version. |
| herald/src/Herald/Command/New.hs | Implements new subcommand and fragment filename generation. |
| herald/src/Herald/Command/Init.hs | Implements init subcommand and template/config generation. |
| herald/src/Herald/Command/Batch.hs | Implements batch subcommand including optional commit/tag. |
| herald/src/Herald/Changelog.hs | Changelog section insertion helper. |
| herald/src/Herald/Cabal.hs | .cabal version extraction and replacement. |
| herald/herald.cabal | Cabal package definition including library/exe/test suites. |
| herald/fourmolu.yaml | Haskell formatter configuration. |
| herald/cabal.project | Cabal project file for building herald. |
| herald/app/Main.hs | CLI option parsing and command dispatch. |
| herald/REQUIREMENTS.md | Requirements + tool survey and rationale for creating herald. |
| herald/README.md | User documentation, CLI usage, config sample, and GitHub Actions docs. |
| herald/PLAN.md | Implementation plan/status and module layout documentation. |
| herald/.hlint.yaml | HLint configuration for the herald subproject. |
| herald/.github/workflows/validate-changelogs.yml | Example consumer workflow for validating fragments. |
| herald/.github/workflows/release.yml | Example consumer workflow for automated release PRs. |
| herald/.github/actions/validate/action.yml | Reusable composite action to run validations in PR CI. |
| herald/.github/actions/release/action.yml | Reusable composite action to batch+tag+open release PRs. |
| flake.nix | Add haskell.nix-based build, apps, checks, devShell, hydra jobs for herald. |
| flake.lock | Lockfile updates for new flake inputs and dependency graph. |
| .gitignore | Ignore Cabal build artifacts and Nix result symlinks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| inputs = { | ||
| systems.url = "github:nix-systems/default"; | ||
| haskellNix.url = "github:input-output-hk/haskell.nix"; | ||
| haskellNix.url = "github:carbolymer/haskell.nix/remove-deprecated-pie-hardening"; |
There was a problem hiding this comment.
haskellNix is pinned to a personal fork/branch (github:carbolymer/haskell.nix/...) rather than the upstream input-output-hk/haskell.nix. This increases supply-chain and maintenance risk for the whole flake (builds depend on a non-canonical repo). If this is required, please pin to a specific commit with justification; otherwise switch back to the upstream source.
| haskellNix.url = "github:carbolymer/haskell.nix/remove-deprecated-pie-hardening"; | |
| haskellNix.url = "github:input-output-hk/haskell.nix"; |
There was a problem hiding this comment.
pr upstreaming changes: input-output-hk/haskell.nix#2487
7fbe688 to
660207a
Compare
|
@palas thanks! I've addressed your remakrs. What do you mean by yaml parsing, which place do you mean? I've also addressed your remarks about windows. I've added running of windows tests to hydra: https://ci.iog.io/jobset/input-output-hk-cardano-dev/pullrequest-22 |
8675baa to
4345888
Compare
6c9c311 to
10058c2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 47 changed files in this pull request and generated 11 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
10058c2 to
19b1c0a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 47 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ah, I was thinking you were parsing the config file manually, but the code I saw was for parsing the git config, I had misunderstood. You are indeed using a yaml parser for the config. The potential issue with windows I talked about would be with the interactive bit, but maybe it is not an issue, it is hard to tell from looking at the code alone |
19b1c0a to
ded6e8a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 48 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Pablo Lamela <pablo.lamela@iohk.io>
ded6e8a to
f0fc2fe
Compare
This PR adds
herald- a release aid tool. Goals of herald:What changes?
heraldoperates on changelogs in checked-in files in.changes/, instead of github PR description. The advantage is that this file can be reviewed in the same way as the code.heraldprovidesherald newinteractive command for creating changelog file. If you don't likeherald- no problem - just copy.changes/_TEMPLATE.ymland put your changelog fragment there. Comments there will guide you.heraldis shipped with two reusable github actions:validate- checks if the new changelog fragment is correct, the PR number matches of the PR that introduces it and the projects are selected accordingly to the diff.release- collects changelog fragments, combines them, selects automatic version bump and creates a release PRheraldis configured by.herald.ymlconfiguration file at the top level of the repository.See it in action
Have a look at "Check changelog fragments" workflows in the following PRs:
There's also a release PR produced by release action: carbolymer/cardano-api#4
See sample migration to
herald: IntersectMBO/cardano-api#1164How to review
I'd suggest starting from
README.md.REQIREMENTS.mdspecifies requirements for the tool as well as comparison with existing changelog generation tools.I encourage reviewers to run the tool and try it.
Reviewing tests can also give some insight.
Reviewing codebase may not be that effective, as this is 100% generated from Claude and I focused on the tool behaviours and output rather than producing top quality code.