Skip to content

fix: export missing module functions and add tests#7

Merged
ren0503 merged 1 commit into
masterfrom
fix/ren/6-missing-export-moduie
May 17, 2026
Merged

fix: export missing module functions and add tests#7
ren0503 merged 1 commit into
masterfrom
fix/ren/6-missing-export-moduie

Conversation

@ren0503

@ren0503 ren0503 commented May 17, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@ren0503 ren0503 added this to the Slog Release v1.1.1 milestone May 17, 2026
@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown

Review Change Stack

Summary by CodeRabbit

  • Tests

    • Verified logger inheritance works correctly across module hierarchies.
  • Improvements

    • Logger provider now has public visibility status.

Walkthrough

The PR adds public visibility metadata to the TSLog provider registration and introduces a test that validates logger inheritance across module hierarchy. ForRoot now sets Status: core.PUBLIC on the TS_LOG provider, and TestInherit confirms that child modules can access the logger configured in parent modules.

Changes

Logger Public Visibility and Inheritance

Layer / File(s) Summary
Provider public status metadata
tslog.go
ForRoot assigns Status: core.PUBLIC to the TS_LOG provider's core.ProviderOptions, making the logger accessible to dependent modules.
Inheritance validation test
tslog_test.go
TestInherit creates a parent–child module hierarchy, registers a controller that injects the logger, and asserts that a GET request completes successfully, validating logger inheritance.

🎯 2 (Simple) | ⏱️ ~8 minutes

🐰 A logger marked public, shining bright,
Flows through modules, left and right,
Parent guides the children well,
Inheritance's tale we now can tell! 🌳

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims to 'export missing module functions' but the actual changes only modify provider metadata and add tests; no function exports were added. Revise the title to accurately reflect the changes, such as 'feat: add Status metadata to TS_LOG provider and inheritance test' or similar.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to verify relevance to the changeset. Add a description explaining the purpose of the Status metadata change and the inheritance test to clarify the changeset intent.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ren/6-missing-export-moduie

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tslog_test.go (1)

58-74: ⚡ Quick win

Consider extracting the duplicated controller logic.

The controller function here is nearly identical to the one in createApp() (lines 16-31). Consider extracting the shared logic into a helper function to follow DRY principles and improve maintainability.

♻️ Proposed refactor to extract shared controller
+func testController(module core.Module) core.Controller {
+	ctrl := module.NewController("test")
+	logger := tslog.Inject(module)
+
+	ctrl.Get("", func(ctx core.Ctx) error {
+		logger.Info("Request processed",
+			"http", slog.Group("request",
+				"method", "GET",
+				"path", "/api/users",
+				"status", 200,
+			),
+			"duration_ms", 150,
+		)
+		return ctx.JSON(true)
+	})
+	return ctrl
+}
+
 func createApp() *core.App {
-	ctrlFnc := func(module core.Module) core.Controller {
-		ctrl := module.NewController("test")
-		logger := tslog.Inject(module)
-
-		ctrl.Get("", func(ctx core.Ctx) error {
-			logger.Info("Request processed",
-				"http", slog.Group("request",
-					"method", "GET",
-					"path", "/api/users",
-					"status", 200,
-				),
-				"duration_ms", 150,
-			)
-			return ctx.JSON(true)
-		})
-		return ctrl
-	}
-
 	moduleFnc := func() core.Module {
 		return core.NewModule(core.NewModuleOptions{
 			Imports:     []core.Modules{tslog.ForRoot(slog.NewJSONHandler(os.Stdout, nil))},
-			Controllers: []core.Controllers{ctrlFnc},
+			Controllers: []core.Controllers{testController},
 		})
 	}

 	return core.CreateFactory(moduleFnc)
 }

 func TestInherit(t *testing.T) {
-	ctrlFnc := func(module core.Module) core.Controller {
-		ctrl := module.NewController("test")
-		logger := tslog.Inject(module)
-
-		ctrl.Get("", func(ctx core.Ctx) error {
-			logger.Info("Request processed",
-				"http", slog.Group("request",
-					"method", "GET",
-					"path", "/api/users",
-					"status", 200,
-				),
-				"duration_ms", 150,
-			)
-			return ctx.JSON(true)
-		})
-		return ctrl
-	}
-
 	parentMod := func(module core.Module) core.Module {
 		return module.New(core.NewModuleOptions{
 			Imports: []core.Modules{tslog.ForRoot(slog.NewJSONHandler(os.Stdout, nil))},
 		})
 	}

 	childMod := func() core.Module {
 		return core.NewModule(core.NewModuleOptions{
 			Imports:     []core.Modules{parentMod},
-			Controllers: []core.Controllers{ctrlFnc},
+			Controllers: []core.Controllers{testController},
 		})
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tslog_test.go` around lines 58 - 74, The controller creation in ctrlFnc
duplicates logic from createApp(): extract the shared handler into a single
helper (e.g., makeTestController or registerLoggingRoute) that accepts a
core.Module (or core.Controller) and returns/configures a controller; move the
tslog.Inject(module) and the ctrl.Get(...) handler (the logger.Info call with
slog.Group and ctx.JSON response) into that helper and then call it from both
ctrlFnc and createApp() to remove duplication while keeping
module.NewController, tslog.Inject, and ctrl.Get usage intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tslog_test.go`:
- Around line 58-74: The controller creation in ctrlFnc duplicates logic from
createApp(): extract the shared handler into a single helper (e.g.,
makeTestController or registerLoggingRoute) that accepts a core.Module (or
core.Controller) and returns/configures a controller; move the
tslog.Inject(module) and the ctrl.Get(...) handler (the logger.Info call with
slog.Group and ctx.JSON response) into that helper and then call it from both
ctrlFnc and createApp() to remove duplication while keeping
module.NewController, tslog.Inject, and ctrl.Get usage intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 40d1eb3f-f954-4279-9d53-92b96ec00475

📥 Commits

Reviewing files that changed from the base of the PR and between 7efad59 and aefb89c.

📒 Files selected for processing (2)
  • tslog.go
  • tslog_test.go

@ren0503 ren0503 linked an issue May 17, 2026 that may be closed by this pull request
@ren0503 ren0503 merged commit ef704f3 into master May 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing Export Module

2 participants