refactor: Replace nix codegen with openapi-gen-build #6

This PR transitions my-forge from a manual Nix-based codegen step (just codegen → nix build .#codegen → artifacts in target/generated/forgejo-client) to an automatic build.rs-driven codegen using openapi-gen-build. The generated code lives at crates/my-forge/src/generated/ and is regenerated on every build. The CI and Nix packaging are updated accordingly.

Positive aspects

Cleaner workflow: No manual codegen step needed — cargo build handles everything.
Consistent dependency graph: Switching reqwest to rustls (dropping native-tls/openssl-sys) reduces transitive dependencies and build complexity.
Good error handling in build.rs: Distinguishes "generator not installed" from other failures with clear panic messages.
CI simplification: Removes the separate Nix-based codegen step; the Build step now installs JRE + openapi-generator-cli and builds directly.
Cargo config added: .cargo/config.toml properly registers the forgejo sparse registry for openapi-gen-build.

Issues

Two minor maintainability concerns — neither is blocking:

Fragile workspace root path (crates/my-forge/build.rs:8)
Hardcoded CLI version (.woodpecker/verify.yaml:14)

*🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: changes

# Review Summary This PR transitions `my-forge` from a manual Nix-based codegen step (`just codegen` → `nix build .#codegen` → artifacts in `target/generated/forgejo-client`) to an automatic `build.rs`-driven codegen using `openapi-gen-build`. The generated code lives at `crates/my-forge/src/generated/` and is regenerated on every build. The CI and Nix packaging are updated accordingly. ## Positive aspects - **Cleaner workflow**: No manual codegen step needed — `cargo build` handles everything. - **Consistent dependency graph**: Switching reqwest to `rustls` (dropping `native-tls`/`openssl-sys`) reduces transitive dependencies and build complexity. - **Good error handling in build.rs**: Distinguishes "generator not installed" from other failures with clear panic messages. - **CI simplification**: Removes the separate Nix-based codegen step; the Build step now installs JRE + openapi-generator-cli and builds directly. - **Cargo config added**: `.cargo/config.toml` properly registers the forgejo sparse registry for `openapi-gen-build`. ## Issues Two minor maintainability concerns — neither is blocking: 1. **Fragile workspace root path** (`crates/my-forge/build.rs:8`) 2. **Hardcoded CLI version** (`.woodpecker/verify.yaml:14`) --- *🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: **changes**

clank-bot reviewed

2026-07-30 18:56:09 +00:00

clank-bot left a comment

Code review findings from opencode.

.woodpecker/verify.yaml

					
				@ -19,3 +12,3 @@

				    image: docker.io/rust:1.97-bookworm

				    depends_on: Generate code

				    commands:

				      - version=7.23.0

[style] The openapi-generator-cli version 7.23.0 is hardcoded. This will silently drift from the version tested locally. Consider reading the version from a single source (e.g. a .env file, or Cargo.toml metadata), or at least adding a comment noting where to update it.

[style] The openapi-generator-cli version `7.23.0` is hardcoded. This will silently drift from the version tested locally. Consider reading the version from a single source (e.g. a `.env` file, or `Cargo.toml` metadata), or at least adding a comment noting where to update it.

crates/my-forge/build.rs

					
				@ -0,0 +5,4 @@

				    use openapi_gen_build::Error;

				    let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());

				    let workspace_root = manifest_dir.join("../../");

[style] workspace_root is derived via manifest_dir.join("../../"), which breaks if the crate is relocated. Use std::env::var("CARGO_WORKSPACE_DIR") instead — it is available in build scripts since Rust 1.64 and is the canonical way to locate the workspace root.

[style] `workspace_root` is derived via `manifest_dir.join("../../")`, which breaks if the crate is relocated. Use `std::env::var("CARGO_WORKSPACE_DIR")` instead — it is available in build scripts since Rust 1.64 and is the canonical way to locate the workspace root.

martin added 1 commit

2026-07-30 19:07:24 +00:00

ci: Merge steps

ci/woodpecker/pr/pr-review Pipeline was successful

Details

ci/woodpecker/push/verify Pipeline was successful

Details

cf93bf148a

clank-bot commented

2026-07-30 19:08:47 +00:00

Review Summary

This PR moves the Forgejo API codegen from a separate Nix derivation (nix/codegen.nix + just codegen) into a build.rs script using the new openapi-gen-build crate. The generated code now lives in crates/my-forge/src/generated/ (gitignored) instead of target/generated/forgejo-client. The forgejo-client external crate is replaced by direct crate dependencies (serde, serde_repr, url, tokio-util, chrono/serde). TLS is switched from native-tls/openssl to rustls (via reqwest's default), simplifying cross-compilation and Nix builds. CI is consolidated into a single Verify step that installs JRE/openapi-generator-cli, builds, clippys, and tests. Overall the direction is correct and the changes are well-executed.

Verdict: Approve — two minor comments below.

*🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: approve

# Review Summary This PR moves the Forgejo API codegen from a separate Nix derivation (`nix/codegen.nix` + `just codegen`) into a `build.rs` script using the new `openapi-gen-build` crate. The generated code now lives in `crates/my-forge/src/generated/` (gitignored) instead of `target/generated/forgejo-client`. The `forgejo-client` external crate is replaced by direct crate dependencies (`serde`, `serde_repr`, `url`, `tokio-util`, `chrono/serde`). TLS is switched from `native-tls`/`openssl` to `rustls` (via `reqwest`'s default), simplifying cross-compilation and Nix builds. CI is consolidated into a single `Verify` step that installs JRE/openapi-generator-cli, builds, clippys, and tests. Overall the direction is correct and the changes are well-executed. **Verdict: Approve** — two minor comments below. --- *🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: **approve**

clank-bot reviewed

2026-07-30 19:09:23 +00:00

clank-bot left a comment

Code review findings from opencode.

.woodpecker/verify.yaml

[performance] Consolidating Build, Clippy, and Test into a single serial step loses parallelism. Previously Clippy and Test ran in parallel after Build; now they run sequentially. Consider splitting back into named steps with depends_on for faster feedback and clearer failure output.

[performance] Consolidating Build, Clippy, and Test into a single serial step loses parallelism. Previously Clippy and Test ran in parallel after Build; now they run sequentially. Consider splitting back into named steps with `depends_on` for faster feedback and clearer failure output.

crates/my-forge/build.rs

					
				@ -0,0 +3,4 @@

				    use openapi_gen_build::Error;

				    let manifest_dir = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap());

[style] Path computation manifest_dir.join("../../") is fragile and assumes the crate is exactly two levels below the workspace root. Using CARGO_WORKSPACE_DIR is unstable; consider iterating parent dirs looking for a workspace Cargo.toml, or at least add a comment documenting the assumed layout.

[style] Path computation `manifest_dir.join("../../")` is fragile and assumes the crate is exactly two levels below the workspace root. Using `CARGO_WORKSPACE_DIR` is unstable; consider iterating parent dirs looking for a workspace `Cargo.toml`, or at least add a comment documenting the assumed layout.