From c7afc93e41e04b672e13eba41b9d6f7e2ddde5e2 Mon Sep 17 00:00:00 2001 From: Maxim Egorov Date: Fri, 28 Aug 2026 01:34:09 +0200 Subject: [PATCH] [client] Trim the new comments to the contributing guide's length budget CONTRIBUTING.md caps comments at 90 characters per line and roughly 250 per comment. The three comments added by this PR were over both limits. The test comments also restated their own test names, so they lose that half and keep only the why. --- client/internal/routeselector/routeselector.go | 7 +++---- .../internal/routeselector/routeselector_test.go | 14 +++++--------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/client/internal/routeselector/routeselector.go b/client/internal/routeselector/routeselector.go index bfcf71552..8a64ad316 100644 --- a/client/internal/routeselector/routeselector.go +++ b/client/internal/routeselector/routeselector.go @@ -32,10 +32,9 @@ func (rs *RouteSelector) SelectRoutes(routes []route.NetID, appendRoute bool, al rs.mu.Lock() defer rs.mu.Unlock() - // Validate before mutating: a non-append selection wipes the current selection first, so - // a request naming only unavailable routes would deselect everything the user had and put - // nothing in its place. Requesting no routes at all is a different thing - that's the - // caller asking to deselect all - and still goes through. + // Validate before mutating: a non-append selection wipes the current selection + // first, so a request of only unavailable routes would deselect everything and + // put nothing back. An empty request means deselect all, so it still goes through. var err *multierror.Error available := make([]route.NetID, 0, len(routes)) for _, r := range routes { diff --git a/client/internal/routeselector/routeselector_test.go b/client/internal/routeselector/routeselector_test.go index 3181d90d0..476542f20 100644 --- a/client/internal/routeselector/routeselector_test.go +++ b/client/internal/routeselector/routeselector_test.go @@ -888,12 +888,9 @@ func TestRouteSelector_EnableExitNodeKeepsOtherRoutes(t *testing.T) { assert.True(t, rs.IsSelected("lan2"), "non-exit route must stay selected") } -// TestRouteSelector_SelectRoutes_AllUnavailableKeepsSelection covers the destructive case: a -// non-append selection clears the current selection before applying the requested one, so a -// request naming only unavailable routes used to leave everything deselected while still -// returning an error - a typo in a route ID silently dropped the user's exit node. A request -// with at least one available route keeps applying the valid part (see "Select non-existing -// route" above); this is only about the all-invalid case. +// A non-append selection clears the current selection before applying the requested +// one, so an all-unavailable request used to leave nothing selected while returning +// an error. Requests with at least one available route are unaffected. func TestRouteSelector_SelectRoutes_AllUnavailableKeepsSelection(t *testing.T) { allRoutes := []route.NetID{"route1", "route2", "route3"} @@ -909,9 +906,8 @@ func TestRouteSelector_SelectRoutes_AllUnavailableKeepsSelection(t *testing.T) { } } -// TestRouteSelector_SelectRoutes_EmptyRequestStillDeselectsAll guards the boundary of the check -// above: asking for no routes is the caller deselecting everything, not a failed request, so it -// must keep working. +// Boundary of the check above: an empty request is the caller deselecting everything, +// not a failed lookup, so it must keep working. func TestRouteSelector_SelectRoutes_EmptyRequestStillDeselectsAll(t *testing.T) { allRoutes := []route.NetID{"route1", "route2", "route3"}