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

Semantic versioning for v7 #804

Merged
merged 22 commits into from
Feb 16, 2022
Merged

Semantic versioning for v7 #804

merged 22 commits into from
Feb 16, 2022

Conversation

faddat
Copy link
Member

@faddat faddat commented Jan 29, 2022

Works on: #788

Description

Semantic versioning for osmosis v7


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@faddat faddat changed the title Faddat/semverv7 Semantic versioning for v7 Jan 29, 2022
@faddat faddat mentioned this pull request Jan 31, 2022
5 tasks
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

Should be good to merge once go.mod sdk version is reverted to original state!

go.mod Outdated Show resolved Hide resolved
@ValarDragon
Copy link
Member

I am generally lost as to what is standard for golang projects. Is it to make main with breaking changes with one version higher, or is it with the same last major release.

Also how do golang projects do backports with these version import differences :/

@mattverse
Copy link
Member

From my knowledge its make breaking changes with one version higher.

@mattverse
Copy link
Member

Also how do golang projects do backports with these version import differences :/
@ValarDragon wdym? Can you give an example ?

@faddat
Copy link
Member Author

faddat commented Feb 4, 2022

You know, there's an issue in this, too. I'm going to jump off of main a second time, to avoid it.

@faddat
Copy link
Member Author

faddat commented Feb 4, 2022

@ValarDragon if you still want to use mergify, we need to add it to this repo. I am going to close this one because I think it might add commits of Sunny's not meant for main.

Also seems like some of the wasm tests are trying to import /x/ibc

I think I got halfway through an upgrade of the tests w/ Juno and it should be the same kind of thing so I'll try and fix them.

It turned out to be quite easy to resolve and was not related to the wasm code 🍾

@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2022

Codecov Report

Merging #804 (d495ac5) into main (e7ab669) will decrease coverage by 0.09%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #804      +/-   ##
==========================================
- Coverage   20.30%   20.21%   -0.10%     
==========================================
  Files         191      192       +1     
  Lines       24740    24854     +114     
==========================================
  Hits         5023     5023              
- Misses      18793    18907     +114     
  Partials      924      924              
Impacted Files Coverage Δ
app/upgrades/v4/upgrades.go 100.00% <ø> (ø)
store/legacy/v101/tree.go 77.77% <ø> (ø)
store/tree.pb.go 38.40% <ø> (ø)
v043_temp/address/hash.go 80.00% <ø> (ø)
x/claim/abci.go 0.00% <ø> (ø)
x/claim/client/cli/query.go 34.45% <ø> (ø)
x/claim/client/cli/tx.go 0.00% <ø> (ø)
x/claim/genesis.go 60.00% <ø> (ø)
x/claim/handler.go 0.00% <ø> (ø)
x/claim/keeper/claim.go 64.33% <ø> (ø)
... and 123 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e7ab669...d495ac5. Read the comment docs.

@faddat
Copy link
Member Author

faddat commented Feb 16, 2022

@mattverse this is complete and I've merged the latest main branch now, too.

Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for this @faddat

In the future, we don't want main to have sem-ver of the next major release, but sem-ver for the latest release, since that would make sense for all the backporting that would be happening. Since we have a major upgrade coming up soon, this PR, in particular, makes sense + super helpful!

@@ -0,0 +1,220 @@
package keeper
Copy link
Member

Choose a reason for hiding this comment

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

Lets re-delete this file

Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

Thanks for the in-depth review @mattverse / checking what other projects do as the standard here.

Did a quick skim, and only saw the v7 change + the re-added legacy.go file.

Lets also file an issue to add the semver situation to the contributing.md

@ValarDragon
Copy link
Member

Going to go ahead and merge before this is broken by the next PR

@ValarDragon ValarDragon merged commit bdff1da into main Feb 16, 2022
@ValarDragon ValarDragon deleted the faddat/semverv7 branch February 16, 2022 06:22
@mattverse mattverse mentioned this pull request Feb 16, 2022
4 tasks
@github-actions github-actions bot mentioned this pull request Mar 15, 2024
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
@github-actions github-actions bot mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants