Skip to content

refactor: pre-release quality pass over the library modules - #5

Merged
w0rxbend merged 31 commits into
mainfrom
refactor/pre-release-quality-pass
Aug 31, 2026
Merged

w0rxbend merged 31 commits into
mainfrom
refactor/pre-release-quality-pass

Conversation

@w0rxbend

@w0rxbend w0rxbend commented Aug 31, 2026

Copy link
Copy Markdown
Member

What this does

A pre-release quality pass over the library modules: it removes duplicated code,
renames three parts of the public API so the names match what they do, and
tightens the test and build gates. Net effect on the source tree is
3595 lines added, 5176 removed across 304 files — the library does the same
things with substantially less code.

Nothing here changes behaviour on purpose. Where behaviour does change, it is
a bug fix, and each one is called out below.

Why

This is the last chance to change public names before a first release, because
after a release every rename is a migration cost paid by other people. Three
names were wrong in ways that only show up when you try to use them:

  • Owner and RepoName are the two types you need before you can make any
    request, but they lived in a repositories sub-package, so the first thing
    every user wrote was an import for a package they had no other reason to know
    about.
  • Every listing operation took a parameter literally named pageParams, which
    restates the type (PageParams) rather than saying anything.
  • Repository sub-resource listings were named listBranches, listTags, and so
    on, inside an object that is already called repos — the list prefix
    repeated a fact the call site already established.

The duplication was the other half. scripts/cpd.sh found the same request-
building and JSON-conversion shapes copy-pasted across the client and codec
modules dozens of times, which is how a fix lands in nine of ten places.

How it works

Read it commit by commit; each commit is one logical change and each one builds
and passes tests on its own.

Bug fixes (behaviour changes, all in modules/client)

  • d7afe65CodebergConfig.defaultPageSize was accepted and then ignored on
    the first page of a listing. It is now honoured.
  • bfaac46 — repository Actions operation ids were reported at the top level
    instead of under repos.actions, so the CallContext on an error named the
    wrong operation.

Public API renames (breaking — see the migration table below)

  • bc32fdfOwner and RepoName move to the package root.
  • 31f46e2 — the everyday surface is re-exported from the package root, so a
    single import com.worxbend.codeberg4s.* is enough to start.
  • 8e2d448pageParams becomes params everywhere.
  • 50f5592repos.listBranches becomes repos.branches, and likewise for
    the other sub-resource listings.
  • 2b078fa — new compile-time literal constructors for path identifiers, so
    owner"forgejo" is checked by the compiler and needs no Either handling for
    a value you wrote by hand.

Deduplication

  • 194f708ce6431c — shared request-shape builders on CodebergRequest,
    then the issue, organization and account APIs rebuilt on top of them;
    b517b30 deletes the hand-written builders that turned out to be identical.
  • 774bcd90a2f0e5 — the same treatment in modules/codec: optional-field
    conversion, array elements, the request-body value builders, the instant
    renderer and nested DTO conversion each now exist once.
  • 0037268 — binary calls go through the shared pipeline path instead of a
    parallel copy of it.
  • e7ee18c — one shared positive-id validator replaces the per-type copies.

Test and build gates

  • e78938e — a reflection-based test that fails if any operation exists on the
    Api rail without its attempt twin. The two rails are meant to be the same
    surface; now they cannot silently drift.
  • 4f3e289 — one stub-backend harness shared by the API suites.
  • 35683f5 and 29a7e94 — ScalaCheck property suites are tagged automatically
    in PropertyBase and excluded from every unit gate, so the fast gate stays
    fast and property tests stay opt-in.

Migration

Before After
import …codeberg4s.repositories.{Owner, RepoName} import com.worxbend.codeberg4s.*
client.repos.listBranches(owner, name) client.repos.branches(owner, name)
client.repos.listTags(…) client.repos.tags(…)
…(owner, name, pageParams = p) …(owner, name, params = p)

All four are compile errors if missed, not silent behaviour changes.

How to test it

./verify.sh

Passes in 41s here: format check, Scalafix, compile under -Werror, 3604 unit
tests, architecture boundary check, and the coverage gate — domain 99.90%,
core 94.71%, codec 95.14% statement coverage, all above their PLAN.md floors.

To confirm the rail-parity test actually bites, delete one attempt method and
re-run ./mill modules.client.test; AttemptParitySuite names the missing one.

Notes for reviewers

  • verify.sh records duplication as a baseline count rather than a threshold —
    the comment in the file explains why, and the number is measured, not chosen.
    This branch should let that number drop; lowering it is worth a follow-up.
  • modules/it and the property suites remain outside the gate on purpose (they
    need Docker or the live network, and property runs are slow).
  • The rebase is onto current main, so the site redesign from feat(site): restyle the documentation site dark-first #3 is already
    underneath this and no site files conflict.

…default

CodebergConfig.defaultPageSize was documented and tested but never read
by production code: every listing takes explicit PageParams, and the
only ready-made starting window, PageParams.First, hardcodes
PageSize.Default. A caller who configured a different size on a
self-hosted instance got the library default anyway unless they built
their own PageParams by hand.

CodebergClient now exposes val firstPage, page one at the configured
defaultPageSize, which is where listings and PageWalk should start.
PageParams.First stays as the config-free constant for code with no
client in hand, since the domain module cannot see any client's
configuration. README, the pagination examples, the PageWalk Scaladoc
and the self-hosted guide now start from client.firstPage, and
CodebergClientSuite pins the wiring.
ADR-0005 promises that the convenience rail and its Attempt mirror
cannot disagree, but the 38 nested Attempt classes are written by hand
and nothing checked the promise. Each mirror method is one more place a
rename, an added parameter, or a new operation can silently miss.

Add AttemptParitySuite, which holds an explicit registry of every
(Api, Api.Attempt) pair and, via java.lang.reflect, asserts that every
public Future-returning rail method has a mirror with the same name and
erased parameter list, that the mirror declares nothing extra, and that
the mirror's generic return type is exactly the rail's result wrapped
in Either[CodebergError, _]. The registry itself is kept honest by
walking the API-typed accessors reachable from CodebergClient, so a new
group cannot land without joining the check. Runs in the normal client
test suite; the mirrors stay hand-written per ADR-0005.
Repository-scoped Actions operations reported bare ids such as
actions.runners.delete, while the organization- and user-scoped
variants of the same endpoints already report orgs.actions.* and
users.account.actions.*. Because identical leaves exist in all three
scopes, telemetry could not tell which scope a call belonged to.

Prefix every operation id in RepositoryActionApi and ActionDownloadApi
with repos., and update the one core test that pinned the old literal.
Operation ids are an observability contract, so this is the last
moment to change them: the library is unreleased and nothing external
depends on the old spellings yet.
Running the project formatter rewrapped two Scaladoc paragraphs that were
committed unformatted in the previous change. No code or documentation
content changes; only line breaks move.
RepositoryApi named its sub-resource listings listBranches, listTags,
listCommits, listReleases, listTopics, and listForks, while the other
API groups name such listings with a bare noun (OrganizationApi.teams,
UserApi.followers). Rename all six, on both the exception rail and the
Attempt mirror, so a listing of a sub-resource is always the plural
noun and the name `list` is reserved for listing the resource itself.

IssueApi.listComments/listLabels/listMilestones keep their prefixed
names: on that class the bare nouns are already taken by the comments,
labels, and milestones sub-API accessors, so the rename cannot apply
there without a collision.

Call sites in the test suite, README, and site docs are updated. The
library is unreleased, so no deprecation cycle is needed.

BREAKING CHANGE: RepositoryApi.listBranches/listTags/listCommits/
listReleases/listTopics/listForks (and the same methods on
RepositoryApi.Attempt) are renamed to branches/tags/commits/releases/
topics/forks. Replace each call with the bare-noun name; signatures
are otherwise unchanged.
The paging window parameter was named `page` in roughly half of the
listing methods and `params` in the other half, sometimes mixed within
a single file. Scala callers can pass arguments by name, so a parameter
name is part of the public API and the inconsistency forces callers to
check each signature before writing `params = ...` or `page = ...`.

Standardise on `params` everywhere — public methods, Attempt mirrors,
and private request helpers. `params` is the more accurate name:
PageParams carries both the page number and the page size, whereas
`page` reads as if it were the number alone. RepositoryWikiApi.page,
the single-wiki-page read, keeps its name — it is a method, not a
PageParams parameter.

The library is unreleased, so no deprecation cycle is needed.

BREAKING CHANGE: listing methods that declared `page: PageParams` now
declare `params: PageParams`. Positional call sites are unaffected;
named-argument call sites must say `params = ...` instead of
`page = ...`.
Owner and RepoName lived in com.worxbend.codeberg4s.repositories, but
they are not repository-group types: the issues, pulls, notifications,
organizations, users, and actions APIs all import them, because almost
every endpoint path starts with owner/name. A cross-cutting identifier
belongs in the root package, next to PathSegment, the validator both of
them delegate to.

This moves the two opaque types (and their test suites) up one
directory into com.worxbend.codeberg4s and mechanically rewrites every
import and fully-qualified Scaladoc link across the domain, codec,
client, it, and examples modules, the README, and the site guides.
Files that sat in the repositories package itself, and so used the
types without an import before, now import them from the root.

BREAKING CHANGE: imports of com.worxbend.codeberg4s.repositories.Owner
and com.worxbend.codeberg4s.repositories.RepoName must become
com.worxbend.codeberg4s.Owner and com.worxbend.codeberg4s.RepoName.
The library is pre-0.1.0, so no released version is affected.
A first-time caller had to know the library's internal package layout
before writing anything: Auth lives in the auth sub-package, Page,
PageParams, and PageSize in paging, while the client, the config, and
the identifier types sit at the root. The README quick start alone
needed six import lines.

This adds a small, curated export file in the domain module that
re-exports Auth, Page, PageParams, and PageSize into the root package
com.worxbend.codeberg4s. With Owner and RepoName now living at the
root too, a single 'import com.worxbend.codeberg4s.*' covers the whole
quick-start flow: build an Auth, construct a client, call an endpoint,
page through a listing.

The list is deliberately short and names types one at a time — no
whole-package re-exports — so every other type keeps exactly one
canonical import from its own sub-package. The README quick start now
uses the wildcard import and explains why it is sufficient.
Nearly every identifier this library takes is written down by the
programmer as a string literal: Owner("forgejo"), BranchName("main"),
RepoName("codeberg4s"). Until now the only way in was `from`, which
returns Either[ValidationError, A] because a value computed at run time
really can be invalid. A literal cannot: it is either valid or it is
not, and which one it is was already decidable while the code compiled.
The result was that every literal dragged a for-comprehension or an
`orFail` helper behind it purely to discharge a failure that could not
happen.

Each of the eighteen identifiers that validate as URI path segments now
also has an `apply` taking a string literal and returning the identifier
itself, no Either around it. An invalid literal is a compile error
naming the field and pointing at the literal. `from` is unchanged and
remains the way in for a value known only at run time — handing one to
the new constructor is itself a compile error, with a message saying so.

The check is a single `inline if` over scala.compiletime.ops.string
.Matches, which asks the compiler whether a literal type matches a
regular expression. There is no macro, and the call folds away to the
literal, so nothing of this reaches the bytecode. That does mean the
rule is now written twice: once as PathSegment's readable if/else chain,
once as a regular expression in the new SegmentLiteral object. Calling
PathSegment at compile time would need a macro, and a macro would need
its own compilation unit; the duplication is the cheaper of the two.
SegmentLiteralSuite pins the spellings together by walking a corpus of
awkward values through both and demanding the same verdict, so a change
to one that is not mirrored in the other fails the build.

One deliberate difference: the literal check refuses surrounding
whitespace where `from` trims it. Silently rewriting what someone typed
is worse than telling them about the typo they can simply fix.

SegmentLiteral is public only because an inline method is expanded in
the caller's own code and everything it mentions has to be reachable
from there. Its Scaladoc says so; nobody should call it directly.
Every API companion in the client module privately defines the same
handful of CodebergRequest constructor calls: a GET with a query, a
mutation with a JSON body, a body-less mutation, a DELETE with and
without a body, and an upload. Fourteen copies of `read`, nine of
`write`, and the body-less mutation under three different names
(`remove`, `bare`, `mutate`) depending on which package you are in.

Duplication of a constructor is not only noise. Every copy repeats
`headers = Nil`, and a copy that quietly forgot to would be
indistinguishable from one that did not until a credential appeared in
a log line. Writing the shapes once, in the companion of the type they
build, is what turns the security contract on CodebergRequest into
something checkable: no builder takes headers, so no call site can set
one.

This commit only adds the builders, marked private[codeberg4s] so they
stay out of the public API. Following commits point the existing call
sites at them and delete the per-package copies.
The issue package had two homes for the same six constructor calls:
IssueRequests, shared by the seven sub-APIs, and a private copy inside
IssueApi, which predates it. Both are now gone in favour of the
builders on the CodebergRequest companion, imported by name so the call
sites still read `read(...)` and `write(...)`.

IssueRequests keeps what is genuinely specific to this group: the path
prefixes. IssueSubscriptionApi keeps its own builder too, renamed from
`bodiless` to `emptyBody`, because it is not the same shape — it sends
a zero-length body where the shared `bodiless` sends none at all, and
two different requests sharing one name is how the wrong one gets
picked.

The upload request builders took a parameter named `upload`, which now
shadows the shared builder of that name; it is `attachment` instead.
OrganizationRequests carried its own read, write and `bare` — the last
being this package's name for the body-less mutation that the issue
package called `remove` and the notification package `mutate`. All
three were the same constructor call. The five API classes here now
import the shared builders from the CodebergRequest companion, and
`bare` becomes `bodiless`, the one name the library uses for that shape.

The rationale `bare` documented — Forgejo's membership, block and
team-assignment routes take no body at all, and answer 400 to bodies
they did not expect — moved with it to the shared builder, so it is
still stated where the decision is made.

OrganizationRequests keeps the two path prefixes and the note on why a
team is rooted at the instance rather than under its organisation.
AccountRequests was the fourth home for the same four constructors. The
five API classes of the account package now import read, write, remove
and removeWithBody from the CodebergRequest companion instead.

The object keeps the part that is specific to this package and is the
reason it exists: the `user` path prefix, and the note that every
endpoint here addresses whoever the configured credentials are, so none
of them may stray into the `/users/{username}` family. Its security
argument for building requests in one place now lives on the shared
builders, which is where it applies to the whole library rather than to
five classes.
Eight API companions defined read, write or remove as byte-identical
copies of the builders now on the CodebergRequest companion. They are
deleted and the shared ones imported by name, so nothing at any call
site changes shape.

This is the bulk of the duplication and none of the judgement: every
one of these copies was the same constructor call under the same name.
The companions whose copies differed in signature are handled
separately.
These five companions had built the same requests as everyone else, but
through signatures of their own, so deleting them meant adjusting call
sites rather than only imports:

- NotificationApi's `mutate` was `bodiless` under a third name; the
  calls now say `bodiless`.
- MiscellaneousApi's `read` took no query at all. Its calls pass `Nil`,
  which is what the shared builder wants and what they meant.
- RepositoryAdminApi's `remove` took an `Option[RequestBody]`, so every
  ordinary delete had to pass `None` and the one delete with a payload
  wrapped it by hand. Those are `remove` and `removeWithBody` now, and
  the odd one out is visible at its call site instead of behind an
  argument.
- RepositoryGitApi's `write` took an `Option[String]` for the same
  reason. Its `DELETE` is a `remove`, and the two real writes pass the
  body directly.
- PullRequestApi built everything through a general `send`. Its reads,
  writes and deletes use the shared builders; what remains is a local
  `post`, for the two calls no shared shape covers — the only mutation
  here with query parameters, and the only one wanting an explicitly
  empty body rather than none.
IssueApi was written before IssueDecoders and IssueRequests existed, so
it carried its own private copies of eight response decoders and four
path builders. Two decoders for the same JSON shape can drift apart
without anything failing to compile, and the issue group had already
begun to split: `search` decoded through IssueDecoders.issues while
`list` decoded through a byte-identical private IssuesDecoder.

IssueApi now takes every decoder from IssueDecoders and every path from
IssueRequests, which is what the seven sub-APIs already did. Two things
were added so that was possible: IssueDecoders.milestones, for the
milestone listing, and IssueDecoders.presentComment, which is the
existing comment decoder without the "an empty body means no comment"
rule — posting a comment never answers 204, so a blank body there is a
malformed response and should be reported as one.

IssueRequests gains labelsPath and milestonesPath, and defines the
single-object labelPath and milestonePath on top of them, so the
collection segment is spelled once. IssueMilestoneApi's inline
`repoPath(...) :+ "milestones"` now goes through milestonesPath too.

No behaviour changes: every replacement decoder and path is the same
value the removed private one produced.
The two-element `page`/`limit` list was written out in sixteen separate
renderers — eleven `*Queries` objects and three private helpers inside
client API classes — with two more one-off spellings for the routes that
take only `page` or that call the size `per_page`.

Each copy was a chance for one endpoint group to drift, and a drifted
copy fails silently: Forgejo answers 200 with the wrong number of items
rather than an error. In particular a `limit` sent without a `page` is
ignored by some endpoints, which returns the entire collection — the
unbounded fetch this library exists to prevent.

`PagingQuery` now owns all three spellings, so each wire name is written
exactly once as rule 4 of `WireConventions` requires. The per-group
`paging` methods stay, because they are the names the API classes call
and the place each group's own remarks belong; their bodies delegate,
and the prose that was repeated eleven times is now a pointer to the one
copy that carries the measurement behind it.

No request changes shape: every existing suite asserting on a rendered
query string passes unchanged.
Scalafmt's import ordering and Scaladoc wrapping had not been applied to
these two files when they landed. Running the formatter now keeps the
realignment out of a later logic diff.
Every endpoint that answers with a bare JSON array was spelling out the
same three parts by hand: read a `Vector[SomethingDto]`, name a lambda
parameter for the decoded vector, and hand `JsonPath.Root` to the DTO's
bulk projection so the per-element error paths read as `[0].name`
instead of being rooted at a field that does not exist.

`WireDecode.vector` now holds that shape in one place, so a list
endpoint names only its DTO and its projection. Envelope-shaped
responses — where the array sits inside an object and the base path is
that object's field, not the root — keep using `WireDecode.of`, and the
new helper's scaladoc says so.

This is a same-behaviour rewrite: `vector` is defined in terms of `of`
with the same root path the call sites passed before, so the decoded
values and every reported failure path are unchanged.
`of` said nothing about which of the two decoding shapes it built, which
mattered once `vector` arrived beside it: a reader scanning a list of
decoders could not tell from the name whether a given one read one JSON
document or an array. `single` and `vector` now read as the pair they
are, and the scaladoc spells out that "single" describes the wire side —
one JSON value, one projection — so an envelope object that happens to
carry a list still belongs to `single`.

`WireDecode` is `private[codeberg4s]`, so this is not a breaking change:
no published name moves and library users have nothing to migrate.
CONTRIBUTING's walkthrough of adding an operation is updated to name
both helpers.
Two endpoint groups each carried a helper that did the same thing:
`repositories.wire.Elements.convert` and `issues.wire.WireElements.at`
both turned an element's position into a JsonPath segment and then
delegated the walk itself to `codec.ArrayElements.convert`. Their own
scaladoc named the duplication and said each was a candidate to move
into `codec` once a second group needed it — which is exactly where the
library ended up, with the pulls, users, organizations and repositories
DTOs importing one or the other more or less at random.

The path-adding step is now an overload of `ArrayElements.convert`
taking the array's own path, so there is one helper instead of three
layered objects, and no call site has to know which group first happened
to need it. Behaviour is unchanged: a single bad element still fails the
whole array, and a failure still reports `$[7].sha` rather than `$`.
`Wire` already covered two of the three ways a DTO field reaches the
domain: demand it, or demand it and run it through a smart constructor.
The third — keep absence, but fail on a value the constructor rejects —
lived in `pulls.wire.PullWire`, reachable only from the pulls package
even though nothing about it is specific to pull requests.

Moving it to `Wire.optional` puts all three shapes in one place, so a
DTO in any endpoint group can express "legitimately absent, and a
present-but-unparseable value is an error" without either re-inventing
the match or, worse, reaching for `Wire.validated` and turning an absent
field into a decoding failure. `PullWire` had no other member, so it is
gone.
`issues.wire.WireInstant` rendered an outgoing timestamp in the one
RFC-3339 spelling Forgejo's Go parser accepts. Its own note said it
should move into `codec` as soon as a second endpoint group needed to
send a timestamp; six groups now do — pulls, notifications, user social,
git data and repository administration all imported it out of the issues
package, which read as an accident rather than a decision.

It becomes `Timestamps.render`, next to the `Timestamps.parse` it is the
counterpart of, so reading and writing a Forgejo timestamp are one
place. The rendering itself is unchanged: `Z` offset, second precision.
`issues.wire.WireNumbers` built the JSON scalars and arrays a request
body is made of, and said in its own note that it should move into
`codec` once a second endpoint group wrote a body. Pulls already
imported it out of the issues package, which is that moment arriving
without anyone acting on it.

It moves to `codec.WireValues`, the `domain → wire` counterpart of the
`Wire` object that reads the other way. The new name drops the "numbers"
claim, which was never true: alongside the identifier and whole-number
builders it also renders arrays of strings. The bodies are unchanged, so
an `int64` field still reaches the wire through `JsonValue.Num` and is
exact past 2^53.
Almost every DTO carried its own private helper to convert an embedded
model — `user.fold(Right(None))(dto => dto.toDomainAt(at.field("user"))
.map(Some.apply))` — written out once per nested field across 27 files.
The shape never varied: absence stays absence, and a present DTO is
converted at the field's own path so a failure reads `$.milestone.title`
instead of `$.title`.

Wire.nested now states that once, next to `required`, `validated` and
`optional`, and the call sites read as one line in the for-comprehension
with no private helper — and no domain-type import — behind them.
Behaviour is unchanged: same paths, same messages, same failure order.
Every identifier Forgejo expresses as a positive int64 was validated by
a copy of the same two lines. Six packages each carried a private helper
object -- issues.NumericId, pulls.PullIds, users.social.SocialIds,
repositories.admin.AdminIds, repositories.access.AccessIds and
repositories.actions.ActionIds -- because each was private to its own
package and therefore unreachable from the next, and seven further
opaque types spelled the check out inline with a local MinValue.

Thirteen copies of one rule is thirteen places to forget the next
clause, which is exactly what happened to path-segment validation before
PathSegment was promoted to the domain module root. This does the same
for the numeric rule: PositiveId lives beside PathSegment, is
private[codeberg4s] so every package can reach it, and the helper
objects and inline copies are gone.

Behaviour is unchanged. A value below 1 is still refused, and the
rejection still names the caller's own field, so a bad issue number is
still reported as "issueNumber" and a bad run id as "runId".
`callBinary` had its own copy of the send/observe/settle sequence —
`binaryAttempt` and `settleBinary` repeated, line for line, what
`attemptOnce` and `settle` already did for a textual call. Two copies of
a cross-cutting rule is two places for it to drift: a fix to how
`Retry-After` is honoured, or to the order telemetry hooks fire in, had
to be made twice and nothing failed if it was made once.

The three methods after the send need only four facts about a response —
its status, the echoed request id, the requested backoff, and the body to
read on the failure path. Those are now named by a private
`ApiPipeline.ResponseFacts[R]`, with one instance for `CodebergResponse`
and one for `BinaryResponse`, and `perform` takes the port method to call
so the same code drives `HttpPort.send` and `BinaryHttpPort.sendBinary`.

No behaviour changes and no published signature changes: `callBinary`
still answers a `BinaryResponse`, still retries as `IdempotentOnly`, and
still hands a successful body back without decoding it.
Every API suite in modules/client carried its own copy of the same
preamble: build a BackendStub, wrap it in an ApiPipeline with an
anonymous config and a prompt retry policy, close the timer afterwards,
and define pathOf/queryOf/methodOf/bodyOf/window/orFail plus the
two-rail failure assertions. Four group-local harnesses (the issue,
organisation, account and social groups) had already been factored out
of parts of it, so the same twenty lines existed in five slightly
different versions and thirteen suites still inlined them by hand.

Copies drift. One copy asserted the query string, another had stopped;
one named the retry-count helper callCount, another attemptsOn; the
"empty" string sttp renders for a bodyless request was spelled out in
three places and reasoned about from scratch in a fourth. None of that
is behaviour under test — it is scaffolding, and scaffolding that
disagrees with itself hides real differences between suites.

ClientSuiteHarness is now the single place that scaffolding lives. It
mixes into a FunSuite, hands out the stub-backend builders, the request
accessors, the pagination window, the smart-constructor unwrapper, the
rails-agree assertion and onPipeline, which builds the pipeline and
releases the timer whatever the outcome. Each suite keeps only what is
specific to it: its fixtures, its stubbed response bodies, and a
one-line onApi that constructs its own API class on that pipeline. The
three surviving group harnesses now extend it and hold nothing but
their group's fixtures and error bodies; SocialApiHarness held nothing
else at all and became the shared harness.

Two renames fall out of the merge. A suite whose Root meant "the prefix
my endpoints hang off" rather than "the instance's API root" now calls
that Endpoint and derives it from the shared Root, so the two ideas
cannot be confused. callCount and failingThenSucceeding are gone in
favour of the harness names attemptsOn and flakyThen.

No test changed what it asserts; the suites are 1757 lines shorter.
`verify.sh` chained the five unit test modules with Mill's `+` separator
and wrote `--exclude-tags=Property` once, after the last one. Mill scopes
the arguments that follow a target to that target alone, so only the last
module in the chain honoured the exclusion: `domain`, `core`, `codec` and
`transport` ran their ScalaCheck suites inside the routine gate.

Nothing went red, which is why it went unnoticed — the properties pass.
It simply meant the gate was slower than intended and that property runs
were not separated from routine verification the way
`docs/CONSTITUTION_MAPPING.md` requires.

The flag is now repeated per target, and a check after the run asserts
the exclusion took effect: an excluded suite reports a total of zero, so
any `*Props` suite finishing with a non-zero total is a property suite
that leaked back into the gate, and the run fails naming it.
Membership of the `Property` tag decided whether a suite ran in the
routine gate, and it rested on each author remembering to write
`.tag(Property)` on every declaration. All three PropertyBase scaladocs
admitted as much: a property written without the tag silently rejoined
the default gate, and nothing would report it.

Each trait now overrides `munitTests()` to add the tag to every test the
subclass declares. munit builds the test list first and filters by tag
afterwards, so this is equivalent to tagging each declaration by hand,
except that it cannot be forgotten. Tags are a `Set`, so the explicit
tags already written stay valid and simply become redundant.
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fbffc439-f863-4cfb-9032-fd6adfd8630b


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Two pages still imported `Owner` and `RepoName` from the
`com.worxbend.codeberg4s.repositories` sub-package, where they no longer
live. Every Scala block on this site is compiled by mdoc against the real
library as part of the site build, so this was not a cosmetic staleness:
it failed the build with nineteen "Not found: type Owner" errors.

The landing page's quick start now uses a single
`import com.worxbend.codeberg4s.*`. That is the point of the root
re-exports — `CodebergClient`, `CodebergConfig`, `Auth`, `Owner` and
`RepoName` all resolve from one line, where the page previously needed
three. Getting Started keeps its imports written out one type at a time,
because a page whose job is to show a beginner where each name comes from
should not hide that behind a wildcard; there, `Owner` and `RepoName`
simply moved onto the existing root import line and `Repository` stays
imported from `repositories`, which is still its package.
@w0rxbend
w0rxbend merged commit 01f5319 into main Aug 31, 2026
4 checks passed
@w0rxbend
w0rxbend deleted the refactor/pre-release-quality-pass branch August 31, 2026 19:08
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