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

Discussion: Version Branches #85

Closed
lance opened this issue Apr 29, 2020 · 19 comments · Fixed by #105
Closed

Discussion: Version Branches #85

lance opened this issue Apr 29, 2020 · 19 comments · Fixed by #105
Labels
status/stalled Issues that have not made progress recently type/discussion Issues that need to be decided/debated/discussed type/enhancement New feature or request type/question Further information is requested

Comments

@lance
Copy link
Member

lance commented Apr 29, 2020

Currently the repository is tagged for each prerelease, mirroring the spec versions as they progressed. However, there are no release branches. This was probably fine prior to 1.0. But now that 1.0 is out, a v1.x branch should be created and maintained so that fixes can be backported to previous versions as we continue to move forward with changes in the repository.

I suggest the following:

  • Create a v1.x branch off of the v1.0.0 tag.
  • Create a v1.x-backport label which can be used to label any PRs that have landed since the 1.0.0 release, indicating they should be considered for backporting to the v1.x branch.
  • As appropriate, create patch releases off of the v1.x branch, tagging the repo on the v1.x branch with v1.0.1, for example, for the next patch release.
  • As appropriate, create minor releases off of the v1.x branch, tagging the repo on the v1.x branch with v1.1.0, for example, for the next minor release.
  • If needed create a v1.1.x branch for minor release patch updates.
  • As appropriate, create patch releases off of the v1.1.x branch, tagging that branch with v1.1.1, for example, for the next patch release in this major/minor combination.
  • Repeat for 2.0.0 and up on their own branches.

This is related to the question raised in #72 where the contributor's guide talks about using the develop branch. That hasn't been done since the 1.0.0 release. We need to decide if we want to move forward with this process. If so, then the develop branch needs to be updated with all recent commits, and set as the primary branch in the repository settings. This setting will ensure that by default, pull requests from contributors are created to land on the develop branch.

My personal feeling is that the develop branch is mostly redundant with master and we don't really need to do that. If contributors can manage rebasing their pull requests with updates from master regularly, this shouldn't be a problem. Though I think this does add some pressure to land PRs quickly. What are other's thoughts on this?

@lance lance added type/enhancement New feature or request type/question Further information is requested labels Apr 29, 2020
@fabiojose
Copy link
Contributor

@lance I am with you lets remove the develop and works with master. As you said, no activity landed to develop for a long time, then it useless to maintain this branch.

@lance
Copy link
Member Author

lance commented Apr 29, 2020

@fabiojose great - I'll leave time for others to comment and will start a PR with some docs around this (or whatever is eventually agreed on in this discussion).

@grant
Copy link
Member

grant commented Apr 30, 2020

This is a really long issue, so I'm sorry if I'm misunderstanding something.

I'm in favor of removing develop and working with master.

I don't think we should create branches for releases, just tags and releases at a tag. This is how the Go SDK and requires much less work. Having multiple branches is a pain.

Branches suggest that there will be future patches on these branches.

@grant
Copy link
Member

grant commented Apr 30, 2020

A separate concern is that the version of this npm module shouldn't be tied to the CloudEvent spec support version. I'm not sure if that's clear, but it's worth restating. The interface for using CloudEvents may change over time even with the same specversion.

mirroring the spec versions

This may have been done in the past but it can't continue. Otherwise, the client (SDK) will be stuck in making non-breaking changes.

It's common to use different version client/SDKs than the CE spec itself.

@lance
Copy link
Member Author

lance commented Apr 30, 2020

@grant yes I agree - versioning of the module is independent of spec version.

Regarding version branches... yes, this assumes that improvements will be made on release versions. I think it's reasonable to assume that the 1.0.0 version of this module will be updated to 1.1.0 or 1.0.1 after version 2.0.0 is out. Release branches help simplify this.

The go SDK seems to follow this pattern: https://github.com/cloudevents/sdk-go/tree/release-1.y.z

That branch seems to be analogous to my proposed v1.x branch, but also integrates my proposed v1.1.x. I'll concede that the patch level branch in my proposal is overkill. But I think it's still important to have the major version branch - how's this?

  • Create a v1.x.y branch off of the v1.0.0 tag.
  • Create a v1.x.y-backport label which can be used to label any PRs that have landed since the 1.0.0 release, indicating they should be considered for backporting to the v1.x.y branch.
  • As appropriate, create patch releases off of the v1.x.y branch, tagging the repo on the v1.x.y branch with v1.0.1, for example, for the next patch release.
  • As appropriate, create minor releases off of the v1.x.y branch, tagging the repo on the v1.x.y branch with v1.1.0, for example, for the next minor release.
  • Repeat for 2.0.0 and up on their own branches.

@grant
Copy link
Member

grant commented Apr 30, 2020

@lance OK, that sounds fine, although a lot of work right now. I'll let you lead that.

Are we unblocked for removing 0.3 support with #99 or is there something else we need to decide before I can create a PR?

@lance
Copy link
Member Author

lance commented Apr 30, 2020

@grant Let's just slow down a tiny bit so that we can ensure everyone agrees on a process, it can be documented, branches can be created, we can ensure CI is set up properly to test these branches, etc. I don't think there's a huge rush to land a naming change right now. Let's do it right.

@grant
Copy link
Member

grant commented Apr 30, 2020

I don't quite understand the proposal above. If you want to create extra branches at this point that are separate from master, then that's fine with me. Seems like a lot of work that won't be very useful to users right now.


With ensuring everyone agrees on the process, can you be more specific? Who do you want to agree? @fabiojose already said he was OK with removing 0.3, not sure about this branch aspect. Who else?

@lance
Copy link
Member Author

lance commented Apr 30, 2020

I don't quite understand the proposal above.

What don't you understand? It's a proposal for managing branches and versions of this module as we move forward.

Sure I can just create a branch and go to town, but I would like to have agreement on this overall strategy first. In what way is creating a branching strategy based on versions a lot of work? And why do you think it will not be useful? I think having clarity on how versions are managed is incredibly useful.

Really doesn't seem like a lot of work to agree on something and have consistency. I'm sorry that you seem to be dissuaded by that.

@grant
Copy link
Member

grant commented Apr 30, 2020

I'd rather have:

  • 1 master branch with the current state of the code
    • New features can use a branch for their respective PRs like we've been doing
  • N number of GitHub releases when we are in a stable state https://github.com/cloudevents/sdk-javascript/releases
    • We can refer to previous releases there.

I don't want to have to patch previous versions that will never get into master. I don't want to have any backports. I don't understand why we want to create so many branches when we can use releases instead.

@grant
Copy link
Member

grant commented Apr 30, 2020

I'd rather have:

  • 1 master branch with the current state of the code

    • New features can use a branch for their respective PRs like we've been doing
  • N number of GitHub releases when we are in a stable state https://github.com/cloudevents/sdk-javascript/releases

    • We can refer to previous releases there.

I don't want to have to patch previous versions that will never get into master. I don't want to have any backports. I don't understand why we want to create so many branches when we can use releases instead.

I think this understanding is a bit old. So long as we don't have an unmanageable level of branches and eventually stop support for old branches, that's fine.

@lance lance added the type/discussion Issues that need to be decided/debated/discussed label May 1, 2020
@helio-frota
Copy link
Contributor

LGTM

@lance
Copy link
Member Author

lance commented May 4, 2020

So long as we don't have an unmanageable level of branches and eventually stop support for old branches, that's fine.

I agree that we need to have a plan for stopping support for older versions. I honestly have no idea how long they should be supported, but I do think that fixing bugs in older versions is a good thing, since people will not always be able to upgrade to newer versions immediately - especially with all of the breaking changes coming down the line - but still need to have bugs addressed.

@grant do you think this can be closed now? Are we in agreement?

Also - would you mind creating your work branches on your own fork so we don't have them proliferating on this repo?

@grant
Copy link
Member

grant commented May 4, 2020

@grant do you think this can be closed now? Are we in agreement?

@lance Yes, it's fine. I really don't think there are enough people developing this to be able to handle the edge case of backporting, but if any maintainer wants to do that, I won't stop it.

Also - would you mind creating your work branches on your own fork so we don't have them proliferating on this repo?

Sure, I can. I'll keep the current PRs here but will open future PRs in a fork.

@lance
Copy link
Member Author

lance commented May 4, 2020

OK thanks.

I don't think it's an edge case. There is currently a bug in the 1.0.0 released version of this module that prevents its usage when using mTLS authentication. Don't you think it would be good to, sooner rather than later, release a 1.0.1 version so that this could be addressed for existing users? How else would you propose we make that release without having a backport branch?

Maybe you're just saying that users should be expected to make a version bump with potential breaking API changes in order to get bugs fixed. I guess that's an approach. It just makes managing upgrades to the module potentially much more difficult for users. It requires code changes instead of a simple dependency bump. Maybe that's not that big of a deal when this module isn't very widely used.

Is that the question I should be asking - what level of support do we expect to provide for old releases? I guess I made some assumptions about that. If we don't really expect to support old versions the whole backport thing is overkill. But if that's the case, maybe we should talk about release cadence, because it'd be nice to release something to deal with this use case sooner rather than later. It's preventing adoption.

I appreciate the debate and hadn't really thought of it that way until now.

@grant
Copy link
Member

grant commented May 4, 2020

My ideal proposal would be to implement this branching strategy once we have a fully functional module that is actually used by folks.
Right now see wee 7k downloads a month.

I would have used this module for Google modules including the Functions Framework which supports CloudEvents and sees ~1,000k downloads a month. If this module was more usable, we could easily adopt it, reduce the duplicative codebase in that module and increase usage of this one.

As soon as it's ready, we can do that. That's just one module of many too. Right now it's not for many reasons.

Another perspective:
Part of our team supports module googleapis, which has 2.3 million downloads a month. We bump the major version for breaking changes often.
https://github.com/googleapis/google-api-nodejs-client

For that project, there is a release strategy but the branches don't matter. I don't see why it's a big deal. I'd rather see thousands of users and depending on this npm module than a fully fleshed out and compliant branch and PR strategy. When it's ready, I can drive a heck ton of usage, create a great experience for Node developers, and talk about this module in keynote presentation (once we're not in a global pandemic and events aren't cancelled).

@lance
Copy link
Member Author

lance commented May 5, 2020

I think it's super cool that you can drive all of that usage! But let's make sure that the small number of people actually using this module have a reasonable expectation that their stuff won't break without a way to deal with it. So, maybe lots of version bumps is the answer. I'm just trying to ensure that as this module progresses existing users can see the benefits of that progression. As it is now, there's no way to know that the big API changes you are suggesting are going to land any time soon, and there actually ARE bug fixes that should be released.

@lance lance changed the title Discussion: Feature Branches Discussion: Version Branches May 5, 2020
@grant
Copy link
Member

grant commented May 5, 2020

OK. I agree, lots of version bumps is the answer for breaking changes. I hope we can proceed with that.

@lance lance added the status/stalled Issues that have not made progress recently label May 6, 2020
@lance
Copy link
Member Author

lance commented May 6, 2020

Closing this since we've reached agreement that frequent releases are going to be the answer for now. We can revisit at a later date if necessary.

@lance lance closed this as completed May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/stalled Issues that have not made progress recently type/discussion Issues that need to be decided/debated/discussed type/enhancement New feature or request type/question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants