Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: migrate golang.org/x/exp to stds #736

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

morlay
Copy link

@morlay morlay commented Feb 12, 2025

fix #735

@morlay morlay requested a review from ndeloof as a code owner February 12, 2025 10:31
@morlay morlay force-pushed the remove-x-exp branch 2 times, most recently from c5e2d13 to f2ac5c1 Compare February 12, 2025 10:33
@@ -120,21 +121,21 @@ func (p *Project) ServicesWithBuild() []string {
servicesBuild := p.Services.Filter(func(s ServiceConfig) bool {
return s.Build != nil && s.Build.Context != ""
})
return maps.Keys(servicesBuild)
return slices.AppendSeq(make([]string, 0), maps.Keys(servicesBuild))
Copy link
Author

Choose a reason for hiding this comment

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

slices.Collect will return nil which break testing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is AppendSeq required here, as maps.Keys already return a []string?

Copy link
Author

@morlay morlay Feb 12, 2025

Choose a reason for hiding this comment

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

it returns iter.Seq[string]

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok got it. Then I don't understand you don't use slices.Collect which does the exact same afaict (https://github.com/golang/go/blob/go1.23.0/src/slices/iter.go#L56-L59)

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/golang/go/blob/go1.23.0/src/slices/iter.go#L58
cause it nil slice as defaults, which will break the testings,
and i have no idea to update testings.

Copy link
Collaborator

Choose a reason for hiding this comment

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

testing uses assert.DeepEqual(t, []string{}, p.ServicesWithBuild()), you can just switch to assert.Equal(t, 0, len(p.ServicesWithBuild()) as nil and empty slice should be considered equivalent

Copy link
Collaborator

Choose a reason for hiding this comment

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

done in #744

@ndeloof
Copy link
Collaborator

ndeloof commented Feb 12, 2025

compose-go declares requirement on go 1.21, while those func have been introduced in 1.23

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.

Remove usage of x/exp
2 participants