Style: fix the findings of the codebase review at a320c3f
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 35/100
- Issue type
- Refactor
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- rust
- Domain
- developer-experience
Research direction
This is an umbrella checklist, so pick one rule and one section, for example the blank line before the closing Ok(...) in tellus/src/backoff.rs:36, after reading the conventions in AGENTS.md. Fix only the listed places, and open a pull request that says Part of #381. The issue asks that this wait until the 0.6 breaking changes land, so check that first. Done means every listed location for that rule is fixed with no behavior change, and the checklist box is ticked.
Written by the indexing model from the issue text.
Description
Style findings from a review of the codebase at a320c3f, against the conventions in AGENTS.md. None of them changes behavior or a public name; the renames of public items are in #346. Best done one rule at a time, each PR Part of this issue, after the breaking changes of 0.6 have landed, since they touch the same files.
Across the workspace
- Blank line between a block and the closing
Ok(...): 35 places, against 168 which follow the rule.tellus/src:backoff.rs:36,host.rs:225,host.rs:371,persistence/in_memory_store.rs:68,:113,cluster/bootstrap.rs:287,cluster/endpoint.rs:1863,cluster/failure.rs:89,cluster/peer.rs:1478,:1697,cluster/reply.rs:671,cluster/sink.rs:171,cluster/transport/quic.rs:642.- Other crates:
tellus-bootstrap-k8s/src/lib.rs:72,:221,tellus-bootstrap-k8s/tests/k8s_seeds.rs:193,tellus-cluster-inspect/src/lib.rs:146,tellus-comparison/benches/frameworks.rs:679,:750,tellus-comparison/src/bin/report.rs:60,:261,:433. - Benches:
tellus/benches/messaging.rs:411,:541,persistence.rs:189,stash.rs:213,:293. - Tests:
tellus/tests/host.rs:492,host_persistence.rs:123,persistence.rs:1531,supervision.rs:546,:589,:728,termination.rs:727,test_context.rs:459.
-
super::imports:tellus/src/cluster/reachability.rs:371is the only one in atellus/srctest module; 41 files there have test modules and the rest import throughcrate::.use super::*intellus-cluster-inspect/src/lib.rs:232,events.rs:327,recorder.rs:118andtellus-cluster-demo-tui/src/model.rs:670,sse.rs:68,ui.rs:465. -
std::sync::mpsc::...written in full attellus/src/cluster/endpoint.rs:3487,tellus/tests/ask.rs:66,:134,:287,:416,:719,supervision.rs:233,:565,:608,termination.rs:85,:387. None of these files has a secondmpscin scope which would force the long path. -
slots_by_keyintellus/src/timers.rs:238is aHashMap<&'static str, usize>. Open: does a map keyed bykeycount as indexed bykeyunder the_by_xrule?
Core (paths under tellus/src unless given in full)
- Item order.
stash.rs:487-488: privateapply_requestssits betweenpub(crate)functions.mailbox.rs:115-123:impl TerminatedHandler for CountedSender<Incoming<M>>lives inmailbox.rs, whileCountedSenderis defined inquota.rs. - Visibility wider than needed.
quota.rs:Quota::reserve(:94),reserve_forced(:107),Reservationandcommit(:124-130).stash.rs:STASHED_FAILED_TO_DROP(:100),Stash::new,settle,note_timer_item,drop_signals_of,hand_back_all,take_all.timers.rs:TimerTable::take_all(:340).
Confidence: Confirmed by reading (grep overtellus/src). - Item docs naming dependents:
watch.rs:8-10(MailboxHandle,ActorContext),quota.rs:205-206,:252-254("lets a host drain what it buffered"),:297-298,stash.rs:613-614(recovered), and the//comment attimers.rs:514(TestContext). - Multi-line docs on non-pub items which explain rather than constrain:
actor_system.rs:127-129,actor_context.rs:924-927,:428-429,:594-595,actor_ref.rs:71-77. Mixed///and//for the same kind of note within one file:actor_context.rs:412,:872against:814,:855. -
tellus/src/quota.rs:299. An empty line separates the doc comment at:297-298from#[test]at:300.
Confidence: Plausible (clippy not run). -
# Errorswording puts a consequence under the heading:actor_context.rs:283(", returning the incoming, so the actor decides what to do with it"),actor_system.rs:77(", which no ordinary termination causes"). -
mailbox.rs:71turns the receive error into anOptionwith.ok()?, whiletry_recvat:88matches the variants.host.rs:366writestokio::spawnwhere the crate otherwise writestask::spawn.
Persistence (paths under tellus/src/persistence unless given in full)
- Item order.
spawn.rs: privateRecovered(154),Settled(160),ReplayError(167),SnapshotError(176) andspawn_event_sourced(323) precede thepub(crate)functions at 348-500;pub(crate) fn apply_events(796) follows the private functions.store.rs:EncodedEvent(113) precedesStoredEvent(127), which contains it; likewiseEncodedSnapshot(137) andStoredSnapshot(152).persistence_id.rs:PersistenceIdSegment(84) precedesInvalidPersistenceId(96).tellus-persistence-postgres/src/lib.rs:is_unique_violation(301) is called byappendbut sits last. - Comments which explain:
spawn.rs:769-770,in_memory_store.rs:81-82,tellus-persistence-postgres/src/lib.rs:97.spawn.rs:187-188has a///on the private macrorun_event_sourced, the only documented non-public item in that file. -
tellus/src/persistence.rs:22-24.pub(crate)onevent_store,snapshot_storeandcodecis not needed: the only readers are inpersistence/spawn.rs, a descendant module. - Longer paths than needed:
tellus-persistence-postgres/src/lib.rs:248(sqlx::migrate::MigrateError),tellus-persistence-postgres/tests/store.rs:53,:59,:65,:357,:358,:435-437(tellus::persistence_tests::...),tellus-persistence-postgres/examples/event_sourced_supervision.rs:60-63.
Cluster: peer and frames (paths under tellus/src/cluster)
- Item order.
frame.rs:111:mod payload_bytesbetweenenum Frame(30) andimpl Frame(164).node.rs:9:pub(crate) struct NodeIdbeforepub struct Incarnation(46) andpub struct ClusterId(60).peer.rs:777:backoff_or_give_upbeforeconnect(798) andwrite_streams(887);admit_inbound(1295) behindread_frames(1142).peer.rs:2808-2809: no blank line between a test andfn addr. - Paths:
frame.rs:137(std::fmt::Result),peer.rs:1809and tests:2132,:2633,:2667(crate::sync::lock(&super::DEAD_LETTERS)),peer.rs:2141(std::future::pending()),:4856-4857(std::time::Instant::now()). - Naming and order. One
Generationhas three names:id(lanes.rs:244),lane_id(lanes.rs:198,peer.rs:64),generation(lanes.rs:135,:152,:158,:165).matching_lane(lanes.rs:277) returns aCountedSender, not a lane.peer.rs:836-848and:1727-1733: generics<S, R>, where clause listsRfirst.peer.rs:507-509puts the receiver before the sender, the tuple at:460-462the other way. -
wire.rs:173-174,:182-183: comments naming "the trait object codec this one replaced", which no longer exists.
Cluster: endpoint and membership (paths under tellus/src/cluster)
- Item order.
membership.rs: privateMemberRepr(89-110) among pub items;pub fn members(302) andpub fn down(315) followpub(crate) enum JoinRound(208);pub(crate) const MIN_FRAME_SIZE(540) is not at the top;Merge,DownOutcome,JoinAdmission(567-596) followpub(crate)functions;impl MemberEntry(955) is separated from its struct (942) byScan(948).endpoint.rs:pub fn start_endpoint(565) andpub fn form(609) precedepub enum Lifecycle(621); privateUncheckedEndpointConfig(402) among pub types; privatefn validate(1808) precedespub(crate) fn get(1866); privateinbound_closed(1395) amongpub(crate)functions. -
impl Traitarguments:endpoint.rs:784-787,membership.rs:384, where the rest of both files uses a type parameter with a where clause. - Visibility wider than needed:
Member::MAX_ENCODED_LEN(membership.rs:83),EndpointInner::offer_snapshot(893),push_reachability(1203),untrack_idle_peers(1451). -
state.rs:32-40orders the fieldsversion, this, cluster, ...;ClusterState::new(:154-162) takesthis, cluster, version, .... - Paths in
endpoint.rstests::2681,:3474(tokio::time::timeout),:3487,:3489,:3497,:3504,:3525(std::thread::spawn). - Comments which explain:
membership.rs:646,endpoint.rs:933-934,:1013-1014,:1616-1617. - Collection names in the singular:
Departures::inboundanddeferred(endpoint.rs:1775-1776),UnreachableWarnings::warned_at(2242).MemberEntry::up()(membership.rs:955-961) readsInstant::now()although its callers are handednow. Missing blank line before the doc comment atendpoint.rs:4088.
Cluster: directory and replies (paths under tellus/src/cluster)
- Item order.
receptionist.rs:51-114: pub free functions precede the pub typesSettlement(119),Subscription(143),ReceptionistError(185).:788,:795:PublishedChangeandSnapshotSourceafter the free functions;:799impl SnapshotSource for Receptionistis 370 lines fromReceptionist(198).:558: privatePartialinside thepub(crate)tier.reply.rs:577,:584:CancellationandProxyOriginfollow thepub(crate)free functions.schema.rs:132:pub(crate) fn newprecedes the pub methods ofPeer. -
tellus/src/cluster/discovery.rs:211-228.Noncelives indiscovery.rs, which does not use it; onlyreply.rs,frame.rsandlanes.rsdo. - Visibility wider than needed:
PendingReplies::discard_ask,cancel_proxy,fail_recipient,AskReplyTags,ReplyToErrorinreply.rs;Receptionist::listing,applied_versionandEntryinreceptionist.rs. - Mechanical nits. (this issue owns part b: the remaining nits,
Key::topicandKey::of_kindincluded (not pub))impl Traitarguments:discovery.rs:38,:42,:50,:173,pubsub.rs:42,receptionist.rs:402.super::path:schema.rs:256.- Bare
type_nameinschema.rs:15,reply.rs:19againstany::type_nameinmanifest.rs:55,discovery.rs:65. watch.rs:314turns an error into anOption;watch.rs:330discards a result.- Comments describing the removed codec:
codec.rs:102,:195,wire.rs:173,:182,reply.rs:880,:1605, and the test names*_matches_the_trait_object_codec. - Bare-noun constructors:
Key::topic,Key::of_kind(discovery.rs:42,:50). - Blank line before the closing
Ok(...):registry.rs:146,reply.rs:849, beside the sweptreply.rs:671.
Cluster: failure detection, formation, bootstrap and transport (paths under tellus/src/cluster unless given in full)
- Style. (this issue owns part b: the remaining style)
- Impl away from its type:
failure.rs:393(type at 134),reachability.rs:98(type at 43). constnot at the top:reachability.rs:36,:41.pub(crate) fn snapshot_chunks(:352) after private items.- Private
Unchecked*types and theirTryFromimpls inside the pub tier:failure.rs:106-117,:348-391,bootstrap.rs:156-198,k8s lib.rs:178-204. impl Into<String>arguments:quic.rs:84,:179,dns lib.rs:60,:70.- Paths:
quic.rs:949,:1018,:1021,:1030,:1308,bootstrap.rs:1083. - Comments which explain:
bootstrap.rs:306,:472.
- Impl away from its type:
Inspect, demo and TUI (paths under tellus-cluster-inspect, tellus-cluster-demo or tellus-cluster-demo-tui, named in the item)
-
tellus-cluster-inspect/Cargo.toml:19,:22.serde_jsonand the tokio featuresmacrosandtimeare used only by tests but are normal dependencies.tellus-cluster-demo/Cargo.toml:18has the sameserde_jsoncase.
Confidence: Confirmed by reading (grep). - Paths:
events.rs:279(std::iter::once),sse.rs:31(std::mem::take),recorder.rs:255,events.rs:502,lib.rs:356,verifier.rs:902,ui.rs:161,:166,:167,:171. -
tellus-cluster-inspect/src/lib.rs:155:trait Sources: Send + Synchas its supertrait bound inline. The demo'sProbing(node.rs:208) uses a where clause. - Item order.
model.rs:531: privateNodeSource::newbeforepub fn label(553).ui.rs:body(43) beforeverifier_strip(74), against the call order indraw.sources.rs:client()is called first and sits last (182).node.rs:log_changes(867) is called frommainbeforerun(531). -
events.rs:175Replay { resumed, anchor, entries }againstrecorder.rs:104Capture { resumed, items, anchor }andevents.rs:263sse(log, items, anchor, ..). - Swallowed errors:
events.rs:254-255(.ok()twice, documented behavior),sources.rs:172-177(polldrops request and decode errors with no log). - Doc comment on a single non-public item:
lib.rs:165-166(Endpoint::snapshot),events.rs:153(read_after).
Examples, benches and comparison (paths under tellus/examples unless given in full)
-
tellus/Cargo.toml:155-156,:213,:218list"persistence"beside"persistence-in-memory", which implies it; the event-sourced tutorial entries list only the latter.work_pullingis the one example without an[[example]]entry. - Paths:
tracing_subscriber::fmt::layer()in all 14init_tracingcopies,shut_down_gracefully.rs:76,secure_with_mutual_tls.rs:94-95,04_remote_watch.rs:217. - Swallowed errors:
shut_down_gracefully.rs:143(.map_err(|_| StatusCode::SERVICE_UNAVAILABLE)),report.rs:374-376. - Item order:
profile.rs:79,benches/messaging.rs:252-258,shut_down_gracefully.rs:157,:169,frameworks.rs:28,:35. - Doc comments on private items, mixed within a file:
cluster.rs:123,:176,:252,:270,:307,:327,:352,:386,:414,remote_scatter_gather.rs:275,:294,benches/messaging.rs:287,:294,:328,:337.///carrying a constraint or a maintenance note:frameworks.rs:498,:711,report.rs:436. - Uniformity across examples.
NonZero..::new(n).unwrap()atretry_writes.rs:27,defer_messages.rs:14,work_pulling.rs:32, against.expect("3 is not zero")in the tutorials.- The context type is spelled three ways:
08_stale_readings.rs:379,retry_writes.rs:78,defer_messages.rs:66. enum Commandfor a plain actor at07_device_manager.rs:144, besideGroupMessage,DeviceMessage,QueryMessage.reserved_addrs(2)literal in clustered 02 and 03, againstNODESin 01, 04, 05.- "## " output prefix only in
cluster.rs,remote_scatter_gather.rs,work_pulling.rs.
Tests (paths under tellus/tests unless given in full)
- Helpers reimplemented per file:
recvandassert_terminates(ask.rs:335,watch.rs:241,termination.rs:317,persistence.rs:1096),next_eventandstop(pipe_to_self.rs:235,timers.rs:1296,stash.rs:753),restart_config(ask.rs:352,stash.rs:646,persistence.rs:1129). - Two reporting idioms:
mpsc::channel(1)withlet _ = self.0.try_send(..), which drops a second report silently, againstunbounded_channelwith.expect("test dropped the receiver"). - Mechanical nits.
super::paths:stash.rs:796,:798,:809,:823,:825.- Three spellings of the std channel:
std::sync::mpsc::...,mpsc as std_mpsc(termination.rs:8),sync::mpsc(timers.rs:227). tokio::spawnatfairness.rs:24,:59,termination.rs:165,timers.rs:323,test_context.rs:278,host.rs:477.impl Traitargument:fairness.rs:83.- Assertion helpers without the
assert_prefix:supervision.rs:296,:308. termination.rs:153tests the cooperative budget, the subject offairness.rs.supervision.rs:172saysmin_backoffandmax_backoff; the fields arebackoff.min()andbackoff.max().- Blank line before the closing
Ok(...):watch.rs:396, beside the swept ones.
Cluster test, justfile and CI
- Style of
tellus/tests/cluster.rs.std::io::stdout()andstd::io::stdin()at 15 places althoughiois imported;std::future::pendingat 361, 645, 693.impl Manifest for Note(1400) away fromstruct Note(1089). The doc at 2168-2171 describes the echo scenario but sits onschema_evolution_scenario. Swallowed errors at 349, 5844, 1337, 1386. Blank line before the closingOk(())at 1706.own()andown_addr(runtime, echo)return different nodes' addresses.
Found by reading the code at a320c3f; nothing was run. Line numbers refer to that commit.
- Dominant language
- Rust
- Stars
- 37
- Forks
- 1
- Avg merge
- 6h 20m
- Merged PRs (30d)
- 135
Getting set up
- Ships a Dockerfile or Docker Compose file
- No pull request template
- No contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from tellus-rs/tellus
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Maintainers usually reply within 1 day
-
documentation question
Difficulty 2/5 1-3 hours Newbie friendliness 60/100
Maintainers usually reply within 1 day
-
good first issue test
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Maintainers usually reply within 1 day
-
tracking
Difficulty 5/5 Over a week Newbie friendliness 8/100
Maintainers usually reply within 1 day
-
bug
Difficulty 3/5 1-2 days Newbie friendliness 45/100
Maintainers usually reply within 1 day
All issues in tellus-rs/tellus
Similar issues
-
[Bug]: opencode_go chat route drops reasoning_effort (even 'none'), while the same endpoint honors it via custom_llm_provider=openaiPossibly taken @arvindkumar-ship-it claimed this today. Openllm translation
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
BerriAI/litellm#51871 · 1 comment ·
Maintainers usually reply within 1 day
-
comp-formats
Difficulty 2/5 Half a day Newbie friendliness 64/100
ClickHouse/ClickHouse#131694 ·
Maintainers usually reply within 1 day
-
cli: `execCargoMetadata` decodes stdout per chunk, corrupting non-ASCII paths across pipe boundariesOpen
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 1 day
-
C-Bug S-Needs-Triage
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
bevyengine/bevy#26558 ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Copperpour/Copperpour#500 ·
Maintainers usually reply within 1 day