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

docs(CONTRIBUTING): refactor, add versioning, remove nightlies #2477

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

thehowl
Copy link
Member

@thehowl thehowl commented Jul 1, 2024

This PR modifies our CONTRIBUTING guide, adding a section on an informal versioning scheme we are temporarily adopting to communicate breaking changes on the Gno repository.

It also removes CI workflows for nightly builds; while retaining the master builds.

After we merge this, I'll proceed to release the 0.1.0 :)

There is a bit of a re-organization of CONTRIBUTING I batched together with these changes, to update some outdated information, link to the documentation where necessary, and provide information on new policies and suggestions adopted in the repository; including disencouraging rebases and giving write access to maintainers.

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests
  • Added new benchmarks to generated graphs, if any. More info here.

@thehowl thehowl self-assigned this Jul 1, 2024
@thehowl thehowl requested review from ajnavarro, moul, a team and jaekwon as code owners July 1, 2024 19:21
Copy link

codecov bot commented Jul 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.11%. Comparing base (9786fa3) to head (7879ef8).
Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2477      +/-   ##
==========================================
- Coverage   63.12%   63.11%   -0.01%     
==========================================
  Files         548      548              
  Lines       78505    78505              
==========================================
- Hits        49559    49552       -7     
- Misses      25601    25606       +5     
- Partials     3345     3347       +2     
Flag Coverage Δ
contribs/gnodev 60.57% <ø> (ø)
contribs/gnofaucet 14.82% <ø> (ø)
gno.land 63.82% <ø> (ø)
gnovm 67.87% <ø> (ø)
misc/genstd 79.72% <ø> (ø)
tm2 62.45% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated
Before that happens, the Gno team has adopted an informal versioning system
for its 0.MINOR.PATCH versions, which works as follows:

- The Gno team releases, weekly-ish, a new PATCH version which will contian a
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- The Gno team releases, weekly-ish, a new PATCH version which will contian a
- The Gno team releases, weekly-ish, a new PATCH version which will contain a

CONTRIBUTING.md Outdated Show resolved Hide resolved
@moul
Copy link
Member

moul commented Jul 1, 2024

I don't get why a manual weekly release is better than an automatic nightly release.

Regarding the PATCH and MINOR. I also don't understand why we currently care about this. Do we have anything that justifies not only updating the patch or just staying with v0.0.1-dev.YYYY.MM.DD?

@thehowl
Copy link
Member Author

thehowl commented Jul 1, 2024

I don't get why a manual weekly release is better than an automatic nightly release.

Regarding the PATCH and MINOR. I also don't understand why we currently care about this. Do we have anything that justifies not only updating the patch or just staying with v0.0.1-dev.YYYY.MM.DD?

The primary need for versioning comes from some recent changes on master, like #2019 and #2382, or the change Milos made removing the timestamp from the transaction, and how these versions interact with users of the gno package, often through gnoclient; like supernova, or a specific tool I wrote for the challenge series. From one day to the next, they broke; the reason was simply that they were using versions of the client and server that were incompatible.

We need a simple system, that can immediately point out to a user when they're using incompatible versions; ie. a counter that keeps track of versions having significant breaking changes. This is not something that is possible with the nightly snapshot versions; ie. it is more understandable that 0.4.1 is incompatible with 0.3.11 rather than v0.1.0-dev.20240612 with v0.1.0-dev.20240528.

The reason to prefer manual releases comes from a few places:

  1. Since the problem partly comes from the go.mods, having a tag or having a go get on @master wouldn't then make much of a difference compared to getting @latest (if we were to have nightly releases every day).
  2. Doing it manually means, for instance, that we can hold off releases for a while as we deem it necessary if we want to batch together a release with several breaking changes.
  3. Releases create a notification email. Dependabot does a good job of filling up my inbox every monday; if I can avoid another meaningless inbox every day I think it's better :)

@moul
Copy link
Member

moul commented Jul 2, 2024

The primary need for versioning comes from some recent changes on master, like #2019 and #2382, or the change Milos made removing the timestamp from the transaction, and how these versions interact with users of the gno package, often through gnoclient; like supernova, or a specific tool I wrote for the challenge series. From one day to the next, they broke; the reason was simply that they were using versions of the client and server that were incompatible.
We need a simple system, that can immediately point out to a user when they're using incompatible versions; ie. a counter that keeps track of versions having significant breaking changes. This is not something that is possible with the nightly snapshot versions; ie. it is more understandable that 0.4.1 is incompatible with 0.3.11 rather than v0.1.0-dev.20240612 with v0.1.0-dev.20240528.

Your suggestion seems premature. I think partial support for multiple versions is worse. Regular rebasing on master is simpler. Manual minor vs patch selection complicates releases and adds confusion while debugging. Regular pulls while we break things often would be clearer (up-to-date VS outdated).

The reason to prefer manual releases comes from a few places:

  1. Since the problem partly comes from the go.mods, having a tag or having a go get on @master wouldn't then make much of a difference compared to getting @latest (if we were to have nightly releases every day).
  2. Doing it manually means, for instance, that we can hold off releases for a while as we deem it necessary if we want to batch together a release with several breaking changes.
  3. Releases create a notification email. Dependabot does a good job of filling up my inbox every monday; if I can avoid another meaningless inbox every day I think it's better :)

1. and 2. seem contradictory. Point 1 implies no one cares about versioning, while Point 2 suggests the versioning cycle needs to be stricter. Who's the target? What's the need?

3. I get the point, but it's not enough to justify the switch.

Nightly tags offer an advantage over @latest, like having Docker images. This was requested by the DevX team.


Edit: I'm fine with replacing nightlies with manual releases. However, I don't understand why you want to introduce the more advanced versioning system at this time.

Copy link
Contributor

@mvertes mvertes left a comment

Choose a reason for hiding this comment

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

Just some few nit-picking. Thank you, nice improvements.

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
@thehowl
Copy link
Member Author

thehowl commented Jul 8, 2024

Discussing with @moul, I convinced him to:

  • Go for manual releases
  • And have them consider a notion of "breaking / non-breaking"; but in order to make this not arbitrary, add a CI workflow that checks for some common "user flows" against what's on the portal loop; like adding a package, or submitting a transaction, and testing if it works. These are CI checks that are completely "allowed to fail" (also because they're bound to happen if the PL is down or for other networking problems); but they can alert about breakages against the current published code, and thus give us a good notion of what's breaking or not

I'm reverting to draft to kick-start developing a few of these checks

@thehowl thehowl marked this pull request as draft July 8, 2024 21:21
Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

LGTM

CONTRIBUTING.md Outdated
version numbers.

Before that happens, the Gno team has adopted an informal versioning system
for its 0.MINOR.PATCH versions, which works as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

@moul
Copy link
Member

moul commented Sep 21, 2024

Discussing with @moul, I convinced him to:

I don't remember what you said, but I don't feel convinced at the moment.

Is there anyone who can share how having v0.1.0 and v0.2.0 has changed their life?

Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

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

need to be convinced again

@thehowl thehowl added the in focus Core team is prioritizing this work label Nov 6, 2024
@thehowl thehowl mentioned this pull request Nov 6, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in focus Core team is prioritizing this work
Projects
Status: In Progress
Status: 📥 Inbox
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants