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

Support for go1.23 #1034

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Support for go1.23 #1034

wants to merge 2 commits into from

Conversation

europaul
Copy link
Contributor

Keeping up with the good tradition to use go's internal APIs and update the dependencies by hand 😄

Doesn't really need to be merged until we move to go 1.23 in go.mod, but can be useful for people using 1.23 locally.

We have internal dependency on testing package that was changed in go
version 1.23

Signed-off-by: Paul Gaiduk <[email protected]>
@europaul europaul requested a review from uncleDecart as a code owner October 17, 2024 15:49
@europaul europaul changed the title Support for go 1 23 Support for go1.23 Oct 17, 2024
@uncleDecart
Copy link
Member

Should be irrelevant once #1037 is merged and escript is deprecated

@europaul
Copy link
Contributor Author

europaul commented Nov 5, 2024

@uncleDecart you're getting rid of the whole escript directory, right?

@uncleDecart
Copy link
Member

@uncleDecart you're getting rid of the whole escript directory, right?

Yes, we merge my PR, move tests to NeoEden and then remove all the old tests completely. It'll take time to do so, though

@europaul
Copy link
Contributor Author

europaul commented Nov 5, 2024

@uncleDecart I just had a look at #1037 and you are keeping escript/go-internal/testscript/testing_1.18.go without basically any changes. Didn't you want to get rid of it?

UPD: oh, you're only gonna get rid of it, once NeoEden tests are merged, right?

@uncleDecart
Copy link
Member

UPD: oh, you're only gonna get rid of it, once NeoEden tests are merged, right?

Right, would be barbaric to just remove what we have before we create new

@europaul
Copy link
Contributor Author

@uncleDecart I guess we can close this one as not relevant anymore, right?

@europaul
Copy link
Contributor Author

@uncleDecart sorry, I just read the previous discussion and now I think that we should rather merge this PR, since NeoEden tests will take a looong time to complete

once they are merged however we call remove both tests/escript/go-internal/testscript/testing_1.18.go and tests/escript/go-internal/testscript/testing_1.23.go

@uncleDecart
Copy link
Member

We will have to bump version in all the GHA files + in go.mod itself if we want to merge this

@europaul
Copy link
Contributor Author

We will have to bump version in all the GHA files + in go.mod itself if we want to merge this

no, we can keep using 1.22.
the problem is only if somebody has their environment setup with go version 1.23: then the device only has go's standard library in v1.23 and this breaks current Eden implementation.

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.

2 participants