Skip to content

refactor(aws): compare the lambda package type with the SDK constant - #1184

Merged
mbevc1 merged 2 commits into
mainfrom
20260915_fix_types
Sep 15, 2026
Merged

mbevc1 merged 2 commits into
mainfrom
20260915_fix_types

Conversation

@mbevc1

@mbevc1 mbevc1 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Cherry pick testing fixes from #1176

Checklist

@github-actions github-actions Bot added go Pull requests that update go code refactor labels Sep 15, 2026
@mbevc1 mbevc1 added tech-investment and removed go Pull requests that update go code refactor labels Sep 15, 2026
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error after 1m 31s —— View job


Follow-up review complete

  • Fetch previous review comments
  • Diff BEFORE..AFTER (fell back to git show f815ef4d, since 8fba57e5 isn't reachable — the newest commit is the only one touching the reviewed lines)
  • Assess status of each previous finding
  • Check changed lines for new bugs/regressions
  • Post inline comments and resolve addressed threads
  • Post summary comment

Result: Both previous findings are resolved by commit f815ef4d, exactly matching the suggested diffs:

  1. internal/aws/aws.go:350 — all three raw pointer derefs (LastModified, CodeSha256, FunctionName) now use aws.ToString, consistent with the existing usage at line 840.
  2. internal/aws/aws_test.go:134LastModifiedTimestamp is now asserted via time.Parse, mirroring TestFormatLambdaLastModified.

No new issues found in the changed lines. Full details posted as a summary comment and two resolved inline replies.

@mbevc1
mbevc1 enabled auto-merge (squash) September 15, 2026 09:00
Comment thread internal/aws/aws.go Outdated
Comment thread internal/aws/aws_test.go
@mbevc1
mbevc1 disabled auto-merge September 15, 2026 09:03
@github-actions github-actions Bot added go Pull requests that update go code refactor labels Sep 15, 2026
A minor bump of a generated AWS client is additive: new model fields and
new enum values. A comparison against a raw string literal does not move
with the SDK.

Lambda's package type is a real SDK enum (lambda/types.PackageType), but
getAndProcessOneLambdaFunc flattened it with string() so that
processOneLambdaFunc could compare it to "Zip". Keep the typed value all
the way down and compare it to types.PackageTypeZip.

ECS has no equivalent: it models Task.lastStatus as a plain string, and
ecsTypes.DesiredStatus is the status a task is heading for, not the one
it last reported. Leave that comparison on the literal, reading the field
through aws.ToString rather than dereferencing a pointer AWS need not
have set.
processOneLambdaFunc's callers dereferenced LastModified, CodeSha256 and
FunctionName directly, even though all three are optional *string fields
on GetFunctionConfigurationOutput. A missing one panics the whole
snapshot goroutine instead of surfacing as an error; a missing
LastModified now degrades into formatLambdaLastModified's existing parse
error, which the caller already handles.

Also assert LastModifiedTimestamp in TestProcessOneLambdaFunc instead of
only the digest, deriving the expectation the same way
TestFormatLambdaLastModified does.
@mbevc1
mbevc1 enabled auto-merge (squash) September 15, 2026 09:08
Comment thread internal/aws/aws.go
Comment thread internal/aws/aws_test.go
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Follow-up review — commit f815ef4

Both previous findings on internal/aws/aws.go / internal/aws/aws_test.go are resolved:

  1. Raw pointer derefs on processOneLambdaFunc call (internal/aws/aws.go:350) — ✅ resolved. LastModified, CodeSha256, and FunctionName now go through aws.ToString, matching the existing pattern already used at line 840.
  2. LastModifiedTimestamp unchecked in TestProcessOneLambdaFunc (internal/aws/aws_test.go:134) — ✅ resolved. The test now asserts it via time.Parse, mirroring TestFormatLambdaLastModified.

No new issues found in the changed lines (internal/aws/aws.go +1/-1, internal/aws/aws_test.go +3).

View job run

@mbevc1
mbevc1 merged commit 8058650 into main Sep 15, 2026
23 of 25 checks passed
@mbevc1
mbevc1 deleted the 20260915_fix_types branch September 15, 2026 09:29
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
kosli-cli 2.41.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre># New features
- Added `kosli attest sbom` command (beta) to report a software bill of materials (CycloneDX JSON/XML or SPDX JSON/tag-value) to an artifact or trail in a Kosli flow. The SBOM file checksum, format, and a parsed summary are recorded; `sbom_format` and `sbom_sha256` are automatically added as annotations.
- Added hidden `--server-side` flag to trail evaluation commands, enabling server-side policy evaluation (experimental, not yet a stable contract).

<!-- Release notes generated using configuration in .github/release.yml at v2.41.0 -->

## What's Changed
* feat(sbom): read CycloneDX and SPDX bills of materials by @AlexKantor87 in kosli-dev/cli#1165
* feat(attest-sbom): add kosli attest sbom by @AlexKantor87 in kosli-dev/cli#1168
* fix(sonar): never send the API token to a redirect target by @mbevc1 in kosli-dev/cli#1170
* fix(snapshot azure): reject zip entries that would extract outside the temp dir by @mbevc1 in kosli-dev/cli#1175
* fix(sbom): read the tool from a CycloneDX services entry by @AlexKantor87 in kosli-dev/cli#1179
* fix(snapshot azure): stop a container spoofing its digest in logs mode by @mbevc1 in kosli-dev/cli#1176
* chore: improve PR follwo-up reviews by @mbevc1 in kosli-dev/cli#1181
* feat(evaluate): evaluate a trail server-side behind a hidden flag by @jumboduck in kosli-dev/cli#1171
* fix: align review turns by @mbevc1 in kosli-dev/cli#1185
* refactor(aws): compare the lambda package type with the SDK constant by @mbevc1 in kosli-dev/cli#1184


**Full Changelog**: kosli-dev/cli@v2.40.1...v2.41.0

</pre>
  <p>View the full release notes at <a href="https://github.com/kosli-dev/cli/releases/tag/v2.41.0">https://github.com/kosli-dev/cli/releases/tag/v2.41.0</a>.</p>
</details>
<hr>

See merge request: Harmonybrew/homebrew-core!20275
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update go code refactor tech-investment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants