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

Clarify branching strategy #751

Merged
merged 5 commits into from
Oct 23, 2022
Merged

Conversation

Art4
Copy link
Contributor

@Art4 Art4 commented Oct 12, 2022

In #743 (comment) we discussed the introduction of a new branching workflow, which is then also described in the README.md. Now that version 1.7.0 is released, it would be the perfect time to do so before merging any other PRs. My proposal is that the branches could look like this:

  • v1.7 (LTS for PHP 5.6)
  • v1.x (Other 1.x versions like 1.8.0, 1.9.0, etc)
  • v2.x (All 2.x versions like 2.0.0, etc.)

Bug fixes or security fixes are created against the latest branch (v1.x or v2.x) and can then be backported to the v1.7 branch.

The current master branch will then no longer be needed.

@mblaney What do you think about the idea of a new branching strategy? The roadmap for that might look like this:

  1. Rename branch master to v1.x (v1.x will be the new default branch)
  2. Create new branch v1.7 from v1.x branch.
  3. Edit README.markdown to explain new branching strategy. (Will be done by this PR.)
  4. Add note in v1.7 branch about LTS. I can create a PR for this.

@jtojnar
Copy link
Contributor

jtojnar commented Oct 12, 2022

What is the benefit of using $major.0 as the development branch over master? That will necessitate renaming the branch in this repo, users’ local checkouts and their GitHub forks every time a new major version is released. Plus we will need to change it in the docs. Why not just keep the latest branch named master and only create 2.x once 3.0.0 is released?

Also, the v prefix is in my experience used for version tags rather than branches (since branches do not usually point to a single concrete version). Though it does not appear to be the case here – version tags so far have had no prefix and branches used words instead of numbers.

Relatedly, another thing to consider is tab completion. When I run a command like git checkout vTab I want it to select a version so ideally branches and versions should not have the same prefix.

@Art4
Copy link
Contributor Author

Art4 commented Oct 12, 2022

What is the benefit of using $major.0 as the development branch over master? That will necessitate renaming the branch in this repo, users’ local checkouts and their GitHub forks every time a new major version is released. Plus we will need to change it in the docs. Why not just keep the latest branch named master and only create 2.x once 3.0.0 is released?

I expect that the v1.x branch will remain the default branch for some time, while in parallel everything will be prepared (removing deprecated code) for release 2.0.0 on a separate branch. So there will be PRs on the v1 as well as on the v2 branch. And there will be backport PRs on the v1.7 branch. Maybe there will be beta or RCs on the v2 branch. Especially in this time I think it is important that the branches can be kept apart well. On which branch should these changes take place if the v1 branch is created after the 2.0.0 release?

At some point the time will come when Release 2.0.0 is published. With my proposal the v2 branch will then become the default. And that's just a setting change on Github.

To be honest, the names of the branches are not relevant to me and I don't have any particular preferences. I am open to other suggestions. What matters to me is THAT there are different branches for different purposes and that the default branch is set sensibly.

How the branches are called is in my experience quite irrelevant for a contributor. For a PR I create a fork, checkout a new branch from the default branch, do my commits and push my branch. Then I create a PR from my branch to the default branch. In most cases I simply don't care what the default branch is called, because I don't commit to the default branch directly.

Also, the v prefix is in my experience used for version tags rather than branches (since branches do not usually point to a single concrete version). Though it does not appear to be the case here – version tags so far have had no prefix and branches used words instead of numbers.

Relatedly, another thing to consider is tab completion. When I run a command like git checkout vTab I want it to select a version so ideally branches and versions should not have the same prefix.

That is the case here. The tags in SimplePie are called 1.7.0, 1.6.0, 1.5.8 and so on. They are not prefixed. Hence my suggestion with the v-prefix for the branches to be able to differentiate them from the tags.
If the tags were called v1.7.0, v1.6.0 and v1.5.8 I would suggest to call the branches 1.7, 1.x and 2.x. 😊 This should avoid confusion on tab completion. And in my experience it works very well. But as I said, I am open to other suggestions.

@jtojnar
Copy link
Contributor

jtojnar commented Oct 13, 2022

Just hit this again with plv8. I needed to send a fix and had a repo cloned locally opened on a branch for the last patch I submitted. So I wanted to checkout the development branch but had to open the repo on GitHub since I could not have guessed that it was called r3.1.

I would mind less if there was a reason but if a project uses one of the standard git workflows, only with branches renamed, it just pointlessly adds a tiny amount of work for infrequent contributors like me. Perhaps I contribute to too many projects (there is 712 directories in my Projects directory. And that is after I pruned it recently.) but it happens enough times a year to make me sufficiently annoyed every time.

I guess your roadmap does not really say if the 2.x and 1.x will be developed in parallel other than backports so I assumed they won’t. Personally, I do not think it makes sense for OSS project to do that unless there is someone financing it, and I would do security critical backports at best.

@Art4
Copy link
Contributor Author

Art4 commented Oct 13, 2022

Just hit this again with plv8. I needed to send a fix and had a repo cloned locally opened on a branch for the last patch I submitted. So I wanted to checkout the development branch but had to open the repo on GitHub since I could not have guessed that it was called r3.1.

I would mind less if there was a reason but if a project uses one of the standard git workflows, only with branches renamed, it just pointlessly adds a tiny amount of work for infrequent contributors like me. Perhaps I contribute to too many projects (there is 712 directories in my Projects directory. And that is after I pruned it recently.) but it happens enough times a year to make me sufficiently annoyed every time.

Very exciting. 😊 I think with so many projects it is easier to get the default branch output via git.

git remote update $REPO_NAME
git remote show $REPO_NAME | grep 'HEAD branch'

// HEAD branch: r3.1

I guess your roadmap does not really say if the 2.x and 1.x will be developed in parallel other than backports so I assumed they won’t. Personally, I do not think it makes sense for OSS project to do that unless there is someone financing it, and I would do security critical backports at best.

It was not the initial plan but a new branching strategy would make this easier. I also think this will be helpful because PRs unfortunately take longer to merge some times.

I have another suggestion for the branch names, which may also fit better with SimplePie's history.

  • one-dot-seven: LTS branch for 1.7
  • master: development of the next minor release of SimplePie 1

If we are ready to start development for SimplePie 2, we can add another branch.

  • one-dot-seven: LTS branch for 1.7
  • master: development of the next minor release
  • two-dot-zero: development of SimplePie 2

After releasing SimplePie 2, master must be renamed to one-dot-nine or similar. And two-dot-zero must be renamed to master.

  • one-dot-seven: LTS branch for 1.7
  • one-dot-nine: maybe security fixes for SimplePie 1
  • master: development of SimplePie 2

But I think switching the master branch this way will create more confusion for contributors.

@mblaney
Copy link
Member

mblaney commented Oct 13, 2022

sorry @Art4 the branching strategies don't sit right with me, I would prefer that development always happened on the trunk and branches started when necessary. I would say keep merging PR's and only back port to one-dot-seven if people are interested in doing that? I think even SimplePie 2 which would remove your deprecated code could fit with that, just need to be clear it's not an upgrade path for legacy users.

Saying that, if there's agreement around a branching strategy and you're keen to take it on, happy for you to make those decisions alongside @jtojnar, @Alkarex and others who are actively contributing.

@Art4
Copy link
Contributor Author

Art4 commented Oct 13, 2022

sorry @Art4 the branching strategies don't sit right with me, I would prefer that development always happened on the trunk and branches started when necessary. I would say keep merging PR's and only back port to one-dot-seven if people are interested in doing that? I think even SimplePie 2 which would remove your deprecated code could fit with that, just need to be clear it's not an upgrade path for legacy users.

Thank you for your answer. I have no problem with your decision. If you create a branch one-dot-seven on which fixes can be backported for SimplePie 1.7, that would be good.

Saying that, if there's agreement around a branching strategy and you're keen to take it on, happy for you to make those decisions alongside @jtojnar, @Alkarex and others who are actively contributing.

I would just like to clarify and document one thing in this PR: What development is taking place on the master branch? Is it only working on the next minor version? And bugfixes for already released minor versions have to be done on a separate branch?

Scenario: When SimplePie 1.8.0 is released, SimplePie 1.9 is developed on master. But if an urgent bugfix is needed in the meantime, where will SimplePie 1.8.1 be created? Will you create a branch one-dot-eight for it first?

@Art4 Art4 changed the title New branching strategy Clarify branching strategy Oct 13, 2022
@Art4
Copy link
Contributor Author

Art4 commented Oct 13, 2022

I've renamed the branch names in the PR.

@mblaney
Copy link
Member

mblaney commented Oct 19, 2022

thanks @Art4 I'm happy to merge this PR. All development can take place on master, I've created one-dot-seven for anything that needs backporting. For your scenario yes I believe releases will need to be tagged off the branch they're on, so even now a 1.7.1 release would need to be tagged off the one-dot-seven branch and same would apply for 1.8.x once the branch one-dot-eight has been created.

there's a few PRs that could now be merged to master if we're happy to continue, but will wait for any further comments.

@Art4
Copy link
Contributor Author

Art4 commented Oct 19, 2022

thanks @Art4 I'm happy to merge this PR. All development can take place on master, I've created one-dot-seven for anything that needs backporting. For your scenario yes I believe releases will need to be tagged off the branch they're on, so even now a 1.7.1 release would need to be tagged off the one-dot-seven branch and same would apply for 1.8.x once the branch one-dot-eight has been created.

Thank you very much for clarifying this @mblaney. 👍 I will try to document this strategy in the README.markdown in general. Because I'm not a native speaker I highly appreciate change requests in my wording. 😊

I think we can remove the WordPress context because there will be no "active" LTS, but we are open for backport PRs to different PHP versions.

Should I add also the one-dot-three branch for PHP 5.2, or is this branch definitely eol?

README.markdown Outdated Show resolved Hide resolved
@mblaney
Copy link
Member

mblaney commented Oct 21, 2022

Yeah the one-dot-three change is good and patches still welcome there too.

@mblaney mblaney merged commit f85c331 into simplepie:master Oct 23, 2022
@Art4 Art4 deleted the new-branching-strategy branch October 23, 2022 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants