Skip to content

Unify read accessors: get on signals, get_state on states - #3

Merged
tiensonqin merged 1 commit into
devin/declare-melange-dependencyfrom
devin/1790229148-unify-get-accessor
Sep 24, 2026
Merged

tiensonqin merged 1 commit into
devin/declare-melange-dependencyfrom
devin/1790229148-unify-get-accessor

Conversation

@tiensonqin

Copy link
Copy Markdown
Contributor

Summary

Signal.get accepted only 'v state — calling it on a bare 'v signal was a type error, so consumers had to discover Signal.sample for the common case (hit twice while building on LUI; see the LUI problems list, P-3).

Now the read verb is uniform:

  • Signal.get : 'v signal -> 'v — alias of sample (the common case).
  • Signal.get_state : 'v state -> 'v — what get used to do.
  • sample unchanged.

Breaking for Signal.get st callers — the error points at get_state. Updated all internal/test call sites; dune build zero warnings, 19/19 tests green. Stacked on devin/declare-melange-dependency so the melange dep fix stays in place for consumers pinning this sha.

Link to Devin session: https://app.devin.ai/sessions/372c486fe4ae4b0894c194b3ca0509c1
Open in Devin Desktop: https://app.devin.ai/desktop/session/372c486fe4ae4b0894c194b3ca0509c1?variant=devin
Requested by: @tiensonqin

Signal.get previously accepted only 'v state, so Signal.get on a bare
'v signal was a type error and callers had to discover Signal.sample.
Now:

- Signal.get : 'v signal -> 'v   (alias of sample; the common case)
- Signal.get_state : 'v state -> 'v  (former get)

sample is unchanged. Callers of get on a state switch to get_state.
@devin-ai-integration

Copy link
Copy Markdown

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@tiensonqin
tiensonqin merged commit 25bf2d5 into devin/declare-melange-dependency Sep 24, 2026
2 checks passed
@tiensonqin
tiensonqin deleted the devin/1790229148-unify-get-accessor branch September 24, 2026 05:58
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.

1 participant