Skip to content

refactor: remove logger from activity, manifest, datastore, auth, and app delete/uninstall#364

Draft
mwbrooks wants to merge 1 commit intomainfrom
mwbrooks-chopping-the-log-1
Draft

refactor: remove logger from activity, manifest, datastore, auth, and app delete/uninstall#364
mwbrooks wants to merge 1 commit intomainfrom
mwbrooks-chopping-the-log-1

Conversation

@mwbrooks
Copy link
Member

@mwbrooks mwbrooks commented Mar 5, 2026

Changelog

  • N/A

Summary

This pull request is part 1 of removing internal/logger package and is focused on the straight-forward use-cases. It will be the largest, but simplest PR.

  • Removes internal/logger dependency from 5 command groups (activity, manifest, datastore, auth, and app delete/uninstall)
  • Migrate functions to return plain data types instead of *logger.LogEvent, with output printed directly by callers

Motivation

The internal/logger package implements an event-driven pub-sub pattern where internal/pkg/ emits named events through a logger and cmd/ subscribes to those events with callbacks that print output. This indirection makes control flow harder to follow and has already been abandoned in newer commands (e.g. cmd/datastore/count.go) which return plain data and print directly.

Test Steps

The ambitious can double-check the output against their production slack installation.

# Setup
$ lack create my-app -t https://github.com/slack-samples/deno-starter-template
$ cd my-app/

# Test: auth list
$ lack auth list
# → Confirm expected output

# Test: manifest validate
$ lack manifest validate
# → Confirm expected output (valid manifest)

# ---

# Deploy the app for all other tests...
$ lack deploy

# ---

# Test: manifest validate (again, this time it will show warnings)
$ lack manifest validate
# → Confirm expected output (shows warnings if on an enterprise)

# Test: activity
$  lack activity -t
# → Confirm expected output
# → Ambitious can execute a trigger to display more output

# Test: datastore put 
$ lack datastore put --datastore SampleObjects '{"item": {"object_id": "42", "description": "Create a PR", "status": "Done"}}'
# → Confirm expected output

# Test: datastore bulk-put
# Test: datastore get
# Test: datastore bulk-get
# Test: datastore query
# Test: datastore update
# Test: datastore delete
# Test: datastore bulk-delete

# Test: app uninstall
# Test: app delete

# Cleanup
$ cd ..
$ rm -rf my-app/

Requirements

@mwbrooks mwbrooks added this to the Next Release milestone Mar 5, 2026
@mwbrooks mwbrooks self-assigned this Mar 5, 2026
@mwbrooks mwbrooks added code health M-T: Test improvements and anything that improves code health semver:patch Use on pull requests to describe the release version increment labels Mar 5, 2026
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 71.29630% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.96%. Comparing base (c449532) to head (83f47e1).

Files with missing lines Patch % Lines
internal/pkg/apps/uninstall.go 0.00% 7 Missing ⚠️
cmd/manifest/validate.go 57.14% 5 Missing and 1 partial ⚠️
internal/pkg/apps/delete.go 50.00% 4 Missing ⚠️
internal/pkg/manifest/validate.go 57.14% 3 Missing ⚠️
cmd/platform/activity.go 0.00% 0 Missing and 1 partial ⚠️
internal/pkg/datastore/bulk_delete.go 66.66% 1 Missing ⚠️
internal/pkg/datastore/bulk_get.go 66.66% 1 Missing ⚠️
internal/pkg/datastore/bulk_put.go 66.66% 1 Missing ⚠️
internal/pkg/datastore/delete.go 66.66% 1 Missing ⚠️
internal/pkg/datastore/get.go 66.66% 1 Missing ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #364      +/-   ##
==========================================
- Coverage   65.06%   64.96%   -0.11%     
==========================================
  Files         215      215              
  Lines       18179    18024     -155     
==========================================
- Hits        11829    11710     -119     
+ Misses       5254     5222      -32     
+ Partials     1096     1092       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code health M-T: Test improvements and anything that improves code health semver:patch Use on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant