refactor: move internal/pkg/version to internal/version#428
refactor: move internal/pkg/version to internal/version#428
Conversation
mwbrooks
added
code health
mwbrooks
force-pushed
the
mwbrooks-refactor-internal-pkg-version
branch
from
82423b7 to
dacfb62
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #428 +/- ##
==========================================
- Coverage 69.76% 69.74% -0.03%
==========================================
Files 220 220
Lines 18446 18446
==========================================
- Hits 12869 12865 -4
- Misses 4407 4408 +1
- Partials 1170 1173 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mwbrooks
force-pushed
the
mwbrooks-refactor-internal-pkg-version
branch
from
dacfb62 to
c082ce3
Compare
mwbrooks
left a comment
mwbrooks
left a comment
There was a problem hiding this comment.
A few comments for my kind teammates 🥇
There was a problem hiding this comment.
farewell: So long and thanks for all the fish pkg 🐬
| @@ -95,8 +95,7 @@ func NewClientFactory(options ...func(*ClientFactory)) *ClientFactory { | |||
| // TODO: Temporary hack to get around circular dependency in internal/api/client.go since that imports version | |||
There was a problem hiding this comment.
thought: I did a little research and I believe this TODO is was solved by our slackcontext.Version(ctx) improvements.
I don't want to remove this comment in this PR, because it's out-of-scope, but I'll start to work on a follow-up PR that removes this comment and potentially some other cleanup related to this.
There was a problem hiding this comment.
@mwbrooks Do be warned - The versions expected for login methods upstream can be reliant on something related to version prefixes... 👻
I'm optimistic from the slackcontext enhancements as well but wanted to share prior edge known.
| @@ -95,8 +95,7 @@ func NewClientFactory(options ...func(*ClientFactory)) *ClientFactory { | |||
| // TODO: Temporary hack to get around circular dependency in internal/api/client.go since that imports version | |||
There was a problem hiding this comment.
@mwbrooks Do be warned - The versions expected for login methods upstream can be reliant on something related to version prefixes... 👻
I'm optimistic from the slackcontext enhancements as well but wanted to share prior edge known.
| "github.com/slackapi/slack-cli/internal/pkg/version" | ||
| "github.com/slackapi/slack-cli/internal/shared" | ||
| "github.com/slackapi/slack-cli/internal/shared/types" | ||
| "github.com/slackapi/slack-cli/internal/slackerror" | ||
| "github.com/slackapi/slack-cli/internal/style" | ||
| "github.com/slackapi/slack-cli/internal/update" | ||
| "github.com/slackapi/slack-cli/internal/version" |
There was a problem hiding this comment.
🔭 praise: This is a nice diff to find!
2 participants
Changelog
Summary
This pull request refactors
internal/pkg/versiontointernal/version. The motivation is to reduce the usage ofinternal/pkg- all of these packages should eventually find a home outside ofinternal/pkg.While this is a small change, it does touch our build configuration.
Test Steps
Requirements