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

fix: dont change the latest network version #9319

Closed
wants to merge 1 commit into from

Conversation

jennijuju
Copy link
Member

@jennijuju jennijuju commented Sep 15, 2022

dont think newest network version should be nv17 in v1.17.2

(shall only be updated close to v1.18.0 -rc

@jennijuju jennijuju requested a review from a team as a code owner September 15, 2022 05:34
@jennijuju jennijuju changed the title fix: network version fix: dont change the latest network version Sep 15, 2022
@arajasek
Copy link
Contributor

Lol, we had this discussion when you were live :cattyping: on #9267 and you wanted to update it!

I'm not sure whether I agree with this change. The biggest reason to keep it is that it makes (most) itests run with the "latest" code. This forces us to iron out all bugs when we integrate a new network version.

This param is otherwise almost exclusively used for devnets and tests. What's your motivation to not update it?

@jennijuju
Copy link
Member Author

I remember I asked if we should - and the answer was yes. But I regret it lolol.

  • adds confusion to user who reads the code
  • I believe devnet is a bit messed up with this, devnet starts at nv16 but there is a couple things we use latest network version

@geoff-vball
Copy link
Contributor

@jennijuju Then I think we need to fix the things that are using the wrong network version. We shouldn't have to slow down development because of that

@jennijuju
Copy link
Member Author

How does that slow down the development?
And key reason, the newest network version should stay v16 until v1.18.0

@geoff-vball
Copy link
Contributor

@jennijuju Does anyone actually look at the code besides you? :)

The line you changed is autogenerated from the list of network versions in go-state-types. We would have to cut a new release of go-state-types to change it back. We can if you really want us to, but I strongly believe it's not worth our time.

@arajasek
Copy link
Contributor

I believe devnet is a bit messed up with this, devnet starts at nv16 but there is a couple things we use latest network version

@jennijuju That just sounds like an issue we should fix -- can you share what seems bork? Should be a quick fix.

@arajasek
Copy link
Contributor

The line you changed is autogenerated from the list of network versions in go-state-types. We would have to cut a new release of go-state-types to change it back

Nah, we can just stop autogenerating that line

@arajasek
Copy link
Contributor

Killing this in favour of #9351

@arajasek arajasek closed this Sep 21, 2022
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.

3 participants