[search] Update search endpoints #1249
Conversation
internal/boxcli/search.go
Outdated
| } | ||
| fmt.Fprintf( | ||
| cmd.OutOrStdout(), | ||
| "%s results to: %s@%s\n", |
There was a problem hiding this comment.
maybe I don't follow this scenario. This will occur for: devbox search [email protected], right?
And the output will be: [email protected] results to: [email protected] ??? This seems a bit ... odd.
I'm missing something...
| return command | ||
| } | ||
|
|
||
| func printSearchResults( |
There was a problem hiding this comment.
in the Test Plan, it would be good to copy-paste examples of the output from this function. It's hard to imagine in my head...
There was a problem hiding this comment.
I kind of get it...but for future PRs, consider this a request.
| ) | ||
|
|
||
| // FetchResolvedPackage fetches a resolution but does not write it to the lock | ||
| // struct. This allows testing new versions of packages without writing to the |
There was a problem hiding this comment.
how and why would one test new versions of packages?
internal/lock/resolve.go
Outdated
|
|
||
| sysInfos := map[string]*SystemInfo{} | ||
| if featureflag.RemoveNixpkgs.Enabled() { | ||
| sysInfos = buildLockSystemInfos(*packageVersion) |
There was a problem hiding this comment.
why dereference, instead of passing the pointer in?
| func WithVersion(version string) SearchOption { | ||
| return func() string { | ||
| return "&v=" + url.QueryEscape(version) | ||
| func (c *client) Resolve(name, version string) (*PackageVersion, error) { |
There was a problem hiding this comment.
could you add a doc-block comment for what resolve does?
| } | ||
|
|
||
| func execSearch(url string) (*SearchResult, error) { | ||
| func execGet[T any](url string) (*T, error) { |
There was a problem hiding this comment.
👍🏾 I feel my programming brain has gotten too comfortable with golang when a simple thing like templates feels novel again :p
| Version: packageInfo.Version, | ||
| Systems: sysInfos, | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
okay, wow, this is much cleaner than the old code!
Also, a 100 bonus points for having the searcher functions return searcher structs, and transforming them into lock structs here. I was also thinking of doing that.
Summary
This updates search endpoints to use new
v1endpoints. It's a fairly small change, but required modernizing the format we use to unmarshal results. I took advantage to make a few low risk refactoring changes:devpkgutilpackage and incorporated to searcher.devbox updatebug where updating was causing lock files entries to get replaced even if version had not changed. (This causes nixpkgs to change which slows stuff down)How was it tested?
devbox updatedevbox add [email protected]devbox searchwith many values, with and without the--show-allflag