pkg/util should contain no code #15634

thockin · 2015-10-14T15:52:54Z

we should move everything into sub-pkgs.

resouer · 2015-10-16T15:33:26Z

Then what left for util, only doc.go & format.go ?

thockin · 2015-10-16T16:45:01Z

util should be a directory that holds other directories and nothing else.

On Fri, Oct 16, 2015 at 8:33 AM, Harry Zhang notifications@github.com
wrote:

Then what left for util, only doc.go & format.go ?

—
Reply to this email directly or view it on GitHub
#15634 (comment)
.

resouer · 2015-10-17T04:49:56Z

I can help with this

resouer · 2015-10-27T07:39:37Z

@thockin What pkg name should we give to backoff ?

p.s. now the caller is like imageBackOff := util.NewBackOff(resyncInterval, MaxContainerBackOff) , thus backoff.NewBackOff would be wired ...

resouer · 2015-10-27T07:53:06Z

Actually, most of the files shares this problem, another example:

type BoolFlag struct {
    // If Set has been invoked this value is true
    provided bool
    // The exact value provided on the flag
    value bool
}

If we refactor it to package boolflag, the caller will do bootflag.Boolflag. It's so wired.

How to give them a proper package name in these cases? @thockin

Maybe, bool.flag ? ...

thockin · 2015-10-28T00:14:33Z

Are there any other rate or flow-control related util packages?
pkg/util/throttle.go looks like a fit. util.RateLimiter too. I'm not
super-keen on backoff's API, but it's OK.

Why not lump these all together into pkg/util/flow (or something short like
that).

While in there, move the fakes (e.g. util.NewFakeRateLimiter()) to a
testing/ subdir, and make sure comments are good Godoc-style comments and
tests are clear and coverage is close to 100%

On Tue, Oct 27, 2015 at 12:40 AM, Harry Zhang notifications@github.com
wrote:

What pkg name should we give to backoff
https://github.com/kubernetes/kubernetes/blob/master/pkg/util/backoff.go#L30
?

p.s. now the caller is like imageBackOff :=
util.NewBackOff(resyncInterval, MaxContainerBackOff)

—
Reply to this email directly or view it on GitHub
#15634 (comment)
.

thockin · 2015-10-28T00:18:23Z

pkg/util/flag/tristate.go, type Tristate - BoolFlag is a bad name for it.

On Tue, Oct 27, 2015 at 12:53 AM, Harry Zhang notifications@github.com
wrote:

Actually, most of the files shares this problem, for example:

type BoolFlag struct {
// If Set has been invoked this value is true
provided bool
// The exact value provided on the flag
value bool
}

If we refactor it to package boolflag, the caller will do
bootflag.Boolflag. It's so wired.

How to give them a proper package name in these cases? @thockin
https://github.com/thockin

—
Reply to this email directly or view it on GitHub
#15634 (comment)
.

thockin · 2015-12-30T06:12:21Z

On Thu, Oct 29, 2015 at 1:00 AM, Harry Zhang notifications@github.com wrote:

Em .. roughly summarize it here, what do you think?
renamed:    util/atomic_value.go -> util/atomic/value.go
renamed:    util/atomic_value.go -> util/atomic/value.go

As discussed elsewhere - good :)

renamed:    util/configuration_map.go -> util/config/configuration_map.go

This is borderline useless - no comments and no tests. I shake my
fist at you @smarterclayton! We should move this to
cmd/kube-apiserver/app where it is used.

renamed:    util/resource_container_linux.go -> util/container/resource_container_linux.go
renamed:    util/resource_container_unsupported.go -> util/container/resource_container_unsupported.go

We can probably remove the kube-proxy call to this lib, which leaves
just kubelet. What do you think @vishh? This can just move to one of
the kubelet dirs.

renamed:    util/diff.go -> util/diff/diff.go
renamed:    util/flags.go -> util/flag/flags.go

ok

renamed:    util/bool_flag.go -> util/flag/tristate.go

I'd just leave it called "bool_flag.go" unless you also want to fix it
at all callsites.

renamed:    util/throttle.go -> util/flow/throttle.go
renamed:    util/backoff.go -> util/flow/backoff.go

ok

renamed:    util/runner.go -> util/func/runner.go
renamed:    util/runner_test.go -> util/func/runner_test.go

ok

renamed:    util/hash.go -> util/hash/hash.go
renamed:    util/hash_test.go -> util/hash/hash_test.go

ok

renamed:    util/http.go -> util/http/http.go

ok

renamed:    util/line_delimiter.go -> util/io/line_delimiter.go
renamed:    util/line_delimiter_test.go -> util/io/line_delimiter_test.go

ugg, I hate this lib. Fine.

renamed:    util/logs.go -> util/log/logs.go
renamed:    util/trace.go -> util/log/trace.go
renamed:    util/port_range.go -> util/port/port_range.go
renamed:    util/port_range_test.go -> util/port/port_range_test.go

ok

renamed:    util/port_split.go -> util/port/port_split.go
renamed:    util/port_split_test.go -> util/port/port_split_test.go

how about net as the pkg for this?

renamed:    util/crypto.go -> util/secure/crypto.go
renamed:    util/ssh.go -> util/ssh/ssh.go
renamed:    util/ssh_test.go -> util/ssh/ssh_test.go
renamed:    util/cache.go -> util/storage/cache.go
renamed:    util/cache_test.go -> util/storage/cache_test.go

ok

renamed:    util/escape.go -> util/string/escape.go

strings

renamed:    util/string_flag.go -> util/string/string_flag.go

this should go with flags

renamed:    util/umask.go -> util/systool/umask.go
renamed:    util/umask_windows.go -> util/systool/umask_windows.go

This should just be moved alongside the only caller of it.

renamed:    util/template.go -> util/template/template.go
renamed:    util/template_test.go -> util/template/template_test.go

ok

renamed:    util/fake_handler.go -> util/tesing/fake_handler.go
renamed:    util/fake_handler_test.go -> util/tesing/fake_handler_test.go

testing but yes

new file:   util/tesing/fake_rate_limiter.go

should be flow/testing/fake_rate_limiter.go - put test utils under
their "real" packages

renamed:    util/clock.go -> util/timeutil/clock.go
renamed:    util/clock_test.go -> util/timeutil/clock_test.go

why timeutil and not time ?

There's a Fake here that should be in a testing sub-pkg, too.

renamed:    util/uuid.go -> util/uuid/uuid.go

ok

resouer · 2015-12-30T06:25:15Z

Thanks, Tim, I was once planning to post a new design here.

smarterclayton · 2015-12-30T17:23:32Z

I think that was my first Kube commit, so I claim newbish.

resouer · 2015-12-31T14:50:10Z

@thockin Two questions:

Are we talking about removing usage of ResourceContainer in proxy's config? Why it is useless may I ask?
There are still plenty of codes in util.go , how should we deal with them? Find a proper directory (maybe not so easy ... )?

thockin · 2015-12-31T17:06:57Z

proxy normally runs as a container now
one at a time, find better homes. Not easy, but we can just keep
chipping away at it.
On Dec 31, 2015 6:50 AM, "Harry Zhang" notifications@github.com wrote:

@thockin https://github.com/thockin Two questions:

Are we talking about removing usage of ResourceContainer in proxy's
config? Why it is useless may I ask?

There are still plenty of codes in util.go , how should we deal
with them? Find a proper directory (maybe not so easy ... )?

—
Reply to this email directly or view it on GitHub
#15634 (comment)
.

Automatic merge from submit-queue Add flow control pkg minor fix ref #15634 Refactor pkg names in back off related files

Automatic merge from submit-queue Refactor util clock into it's own pkg Continue my work ref #15634

Automatic merge from submit-queue Refactor uuid into its own pkg util/uuid Continuing my work ref #15634 Anyone can review this if he/she wants.

Automatic merge from submit-queue Refactoring runner resource container linedelimiter to it's own pkg Continuing my work ref #15634 Anyone is ok to review this fix.

Automatic merge from submit-queue Merge string flag into util flag Continuing my work on #15634 This refactoring is expected to be completely finished and then I will add a verify scripts in `hack`

xiangpengzhao · 2017-06-21T05:02:51Z

still not be cleant up completely...
/sig api-machinery

@thockin

Automatic merge from submit-queue (batch tested with PRs 48481, 48256) Refactor: pkg/util into sub-pkgs **What this PR does / why we need it**: - move code in pkg/util into sub-pkgs - delete some unused funcs **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #15634 **Special notes for your reviewer**: This is the final work of #15634. It will close that issue. /cc @thockin **Release note**: ```release-note NONE ```

thockin added the kind/technical-debt label Oct 14, 2015

thockin added this to the v1.2-candidate milestone Oct 14, 2015

saad-ali added the team/none label Oct 23, 2015

resouer mentioned this issue Oct 24, 2015

Move atomic_value.go into folder #16237

Merged

resouer mentioned this issue Nov 15, 2015

Add default when latest not claimed #17278

Merged

This was referenced Jan 5, 2016

[WIP] Fix the util package contains too much codes #19283

Closed

Use hashutil to hold hash tools #19331

Merged

Networking should be used to hold network related pkgs #19334

Merged

Create strings pkg to fix util issue #19528

Merged

This was referenced Jan 15, 2016

Replace fake handler using testing pkg #19685

Merged

Replace runtime reference by subdir #19778

Merged

bgrant0607 added the help-wanted label Jan 29, 2016

bgrant0607 modified the milestones: next-candidate, v1.2-candidate Jan 29, 2016

bgrant0607 added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Jan 29, 2016

resouer mentioned this issue Feb 5, 2016

refactor logs into sub pkg #20713

Closed

This was referenced Mar 9, 2016

Add flow control pkg #22733

Merged

Refactor throttle into util sub pkg #22735

Merged

This was referenced Mar 9, 2016

Fix util pkg contains code of boolflag cache configrationmap #22736

Merged

Util pkg refactored into sub pkgs #22838

Merged

Refactor env flag into sub pkg of util #22939

Merged

k8s-github-robot pushed a commit that referenced this issue Apr 11, 2016

Merge pull request #22733 from resouer/flow-control

6a87dba

Automatic merge from submit-queue Add flow control pkg minor fix ref #15634 Refactor pkg names in back off related files

resouer mentioned this issue May 26, 2016

Refactor util clock into it's own pkg #26332

Merged

resouer mentioned this issue Jun 7, 2016

Refactoring runner resource container linedelimiter to it's own pkg #26958

Merged

resouer mentioned this issue Jul 26, 2016

Refactor uuid into its own pkg util/uuid #29621

Merged

k8s-github-robot pushed a commit that referenced this issue Jul 28, 2016

Merge pull request #26332 from resouer/util-debt

eae90a3

Automatic merge from submit-queue Refactor util clock into it's own pkg Continue my work ref #15634

k8s-github-robot pushed a commit that referenced this issue Aug 2, 2016

Merge pull request #29621 from resouer/uuid

01cd7f3

Automatic merge from submit-queue Refactor uuid into its own pkg util/uuid Continuing my work ref #15634 Anyone can review this if he/she wants.

k8s-github-robot pushed a commit that referenced this issue Aug 3, 2016

Merge pull request #26958 from resouer/util-line

24216fe

Automatic merge from submit-queue Refactoring runner resource container linedelimiter to it's own pkg Continuing my work ref #15634 Anyone is ok to review this fix.

bgrant0607 removed the help-wanted label Aug 30, 2016

resouer mentioned this issue Oct 12, 2016

Merge string flag into util flag #34587

Merged

bgrant0607 removed the team/none label Dec 8, 2016

k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label May 31, 2017

0xmichalis added the for-new-contributors label Jun 11, 2017

k8s-ci-robot added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 21, 2017

k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jun 21, 2017

xiangpengzhao mentioned this issue Jun 29, 2017

Refactor: pkg/util into sub-pkgs #48256

Merged

k8s-github-robot closed this as completed in #48256 Jul 19, 2017

resouer mentioned this issue Aug 1, 2017

[code style] verify pkg/util contains no code #49923

Closed

BenTheElder mentioned this issue Feb 25, 2021

Reducing Kubernetes Build Maintenance kubernetes/enhancements#2420

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pkg/util should contain no code #15634

pkg/util should contain no code #15634

thockin commented Oct 14, 2015

resouer commented Oct 16, 2015

thockin commented Oct 16, 2015

resouer commented Oct 17, 2015

resouer commented Oct 27, 2015

resouer commented Oct 27, 2015

thockin commented Oct 28, 2015

thockin commented Oct 28, 2015

thockin commented Dec 30, 2015

resouer commented Dec 30, 2015

smarterclayton commented Dec 30, 2015 via email

resouer commented Dec 31, 2015

thockin commented Dec 31, 2015

xiangpengzhao commented Jun 21, 2017

pkg/util should contain no code #15634

pkg/util should contain no code #15634

Comments

thockin commented Oct 14, 2015

resouer commented Oct 16, 2015

thockin commented Oct 16, 2015

resouer commented Oct 17, 2015

resouer commented Oct 27, 2015

resouer commented Oct 27, 2015

thockin commented Oct 28, 2015

thockin commented Oct 28, 2015

thockin commented Dec 30, 2015

resouer commented Dec 30, 2015

smarterclayton commented Dec 30, 2015 via email

resouer commented Dec 31, 2015

thockin commented Dec 31, 2015

xiangpengzhao commented Jun 21, 2017