Skip to content

refactor: move URL normalization to shared package#26

Merged
rianjs merged 1 commit intomainfrom
refactor/14-url-normalization-to-shared
Jan 29, 2026
Merged

refactor: move URL normalization to shared package#26
rianjs merged 1 commit intomainfrom
refactor/14-url-normalization-to-shared

Conversation

@rianjs
Copy link
Copy Markdown
Contributor

@rianjs rianjs commented Jan 29, 2026

Summary

  • Create shared/url package with NormalizeURL, HasScheme, and TrimTrailingSlashes functions
  • Update jtk to use shared URL normalization in config, API client, and config command
  • Remove duplicate NormalizeURL from jtk/internal/config
  • Remove duplicate hasScheme and trimTrailingSlash from jtk/api

Notes

cfl's NormalizeURL is different (adds /wiki suffix for Confluence) so it remains in place as a method on Config.

Test plan

  • All existing tests pass
  • New unit tests for shared/url package
  • Lint passes

Closes #14

🤖 Generated with Claude Code

- Create shared/url package with NormalizeURL, HasScheme, and
  TrimTrailingSlashes functions
- Update jtk to use shared URL normalization in config, API client,
  and config command
- Remove duplicate NormalizeURL from jtk/internal/config
- Remove duplicate hasScheme and trimTrailingSlash from jtk/api

Note: cfl's NormalizeURL is different (adds /wiki suffix for Confluence)
so it remains in place.

Closes #14
@rianjs
Copy link
Copy Markdown
Contributor Author

rianjs commented Jan 29, 2026

Test Coverage Assessment for PR #26

Summary

This PR extracts URL normalization utilities from jtk into a new shared url package and updates the consumers to use the shared package. The test coverage is excellent.

New Code: shared/url/url.go

Function Coverage Assessment
NormalizeURL 100% Fully covered with 9 test cases
HasScheme 100% Fully covered with 6 test cases
TrimTrailingSlashes 100% Fully covered with 5 test cases

Test quality highlights:

  • Empty string handling tested
  • Both http:// and https:// schemes tested
  • Single and multiple trailing slashes tested
  • URLs with and without paths tested
  • Edge cases like ftp:// and httpx:// schemes tested (should return false)

Changed Code: Consumers

The PR updates three files to use the shared package:

  1. tools/jtk/api/client.go - Replaces inline hasScheme and trimTrailingSlash with url.NormalizeURL
  2. tools/jtk/internal/config/config.go - Replaces local NormalizeURL with shared version
  3. tools/jtk/internal/cmd/configcmd/configcmd.go - Updates import path

Existing tests in config_test.go verify the integration works correctly through TestNormalizeURL (which now tests the shared package) and the various TestGetURL_* tests that exercise normalization in context.

Coverage Numbers

  • shared/url: 100% of statements
  • jtk/internal/config: 80.8% of statements (unchanged by this PR)
  • jtk/api: 51.7% of statements (pre-existing - not affected by this refactor)

Verdict

Adequate coverage. The new shared package has 100% test coverage with comprehensive edge case testing. The refactoring is straightforward - extracting existing functionality into a shared location - and existing integration tests verify the wiring is correct. All tests pass.

No additional tests needed.

@rianjs rianjs merged commit 62dc646 into main Jan 29, 2026
7 checks passed
@rianjs rianjs deleted the refactor/14-url-normalization-to-shared branch January 29, 2026 19:57
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.

refactor: move URL normalization to shared package

1 participant