Repository navigation
Move hill chart dots and post a progress update - #991
Conversation
|
@codex review |
github-actions
Bot
added
commands
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
v0.34.0 adds HillChartsService.CreateVersion (basecamp-sdk#1068), which posts a progress update on a to-do set's hill chart. Nothing else in the CLI breaks. Syncs the vendored MCP model to go/v0.34.0, which serves it as create_hill_chart_version in basecamp mcp, and refreshes the Nix vendorHash.
hillcharts update --dot <todolist-id|name>=<0-100> moves tracked todolists, as updating the hill in Basecamp does; lists left out keep their position. --note is Markdown converted to rich text (- reads stdin), and --notify names who is told, since the API notifies nobody otherwise.
… before reading stdin The API takes subscriptions on their own: the chart is recorded as it stands and the people named are told.
7f74fa3 to
945ef9e
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Copilot was re-requested on this head but never reported: it has been out across basecamp-sdk, basecamp-cli and basecamp-mcp-server since yesterday, and review requests to it don't register. Rob decided to merge without it. CI is green on this head, Codex reviewed it with no findings, and there are no open threads. |
What
Move dots on a hill chart and post a progress update, as updating the hill in Basecamp does:
--dotis repeatable. Each one moves a tracked todolist, named by ID or by name. Names resolve within the todoset, ashillcharts trackdoes. The value splits at the last=, so a name can contain one. Todolists left out keep their position. Two--dots for the same list are refused before the request.--noteis Markdown converted to rich text, like other commands' bodies (--format htmlsends HTML as written; @mentions resolve).-reads stdin.--notifytakes comma-separated names, emails, IDs ormeand sends them assubscriptions. Without it the API notifies nobody else.--notifyalone posts the chart as it stands and tells those people.hill_chart_versionschema from Read a hill chart's progress updates #990. Breadcrumbs lead to the hill chart, the updates list and the update's comments.--notify, a non-numeric--todoset, or nothing to post. A 422 from Basecamp (an untracked list, say) comes back as a validation error with its message.For Hill chart writes in the API. The endpoint is added by bc3#14214, and the SDK operation
HillChartsService.CreateVersionby basecamp/basecamp-sdk#1068.bc3#14214 has merged and basecamp-sdk v0.34.0 ships the operation. This branch is rebased onto main, which now has #990, and pinned to that release.
SDK pin
go.modpinsgithub.com/basecamp/basecamp-sdk/goto the releasedv0.34.0(make bump-sdk REF=v0.34.0). The MCP model is synced fromgo/v0.34.0(f8902736), which addscreate_hill_chart_version: the catalog goes from 286 to 287 operations and the snapshot is updated.nix/package.nixvendorHash is regenerated withmake update-nix-hash, and the Docker Nix build of the new hash passes.sdk-provenance.jsonreads v0.34.0.API-COVERAGE.md(221 of 221) and.surfaceare unchanged from the earlier review.Testing
make checkpasses locally on the head commit, with golangci-lint v2.11.1 as in CI.=), Markdown,--format htmland stdin notes,--notifywith and without dots, todoset resolution from--inwith breadcrumbs, duplicate lists, a 422 from the server, and 11 flag mistakes refused before any request.e2e/hillcharts.bats19/19, including 4 new tests.hillcharts update --dot Launch=60 --note "Over the hill: **design is settled**" --notify meexited 0.GET .../hills/versions/:id.jsonandGET .../hill.jsonboth show Launch at 60, the note as<strong>HTML, and one subscriber.hillcharts updateslists the new update first.--todoset 999999999got bc3's 404 asnot_found, with exit 2. An untracked list by ID got bc3's 422 asvalidation,Not tracked on the hill chart: ..., with exit 9. Both reached the server.--dot Launch=150was refused as a usage error before any request.create_version, against the same server, moved Launch to 75 (read back from the API) and raisedValidationErroron the untracked list.hill-chart-updates-apif0f63e7d438) and read each result back from the API:GET /todosets/:id/hill.jsonandGET .../hills/versions/:id.jsonshow both positions and the note as<strong>/<em>HTML.--dotleft the other list where it was. A note-only post moved nothing.--notify, the update has 1 subscriber (the creator).--notify <person id>gave 2: that person and the creator.{"code":"validation","error":"Not tracked on the hill chart: ..."}with exit 9.updates listshows the new updates newest first.--styledrenders the dots.hillcharts update posts a progress update), which needs a live account. It re-posts a tracked dot at its current position, so the chart is left as it was.Review
A cross-family review of the branch found two issues, both fixed in 7f74fa3:
--notifywith no--dotor--notewas refused as "No update fields specified", though the API acceptssubscriptionsalone (it records the chart and notifies). It now posts. There's a test.--todosetwith--note -read stdin before the todoset was rejected, so a live pipe would hang. The todoset ID is now checked with the other flags, before stdin. There's a test.Codex reviewed 7f74fa3 and found no issues. The rebase onto main and the repin to v0.34.0 change no command code: 3ec106b and 945ef9e are those two commits, replayed unchanged.