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

Simplify branching and documentation strategy #1238

Open
hiddeco opened this issue Jul 4, 2023 · 11 comments
Open

Simplify branching and documentation strategy #1238

hiddeco opened this issue Jul 4, 2023 · 11 comments

Comments

@hiddeco
Copy link
Member

hiddeco commented Jul 4, 2023

Currently, this repository follows a branching strategy with master and develop branches. Pull requests are created against the develop branch for ongoing development, and only when a release is made, changes are merged into master. However, this approach has some drawbacks, such as an outdated README.rst for small documentation fixes until a release is made and confusion for new contributors who create pull requests against the wrong branch (master).

My proposal would be an alternative approach that involves simplifying the branching strategy and handling documentation differently (getsops/community#9), while also renaming master to main (getsops/community#16).

Proposed alternative

  • A single main branch (renamed from develop).
  • Most of README.rst moved to e.g. docs/.
  • Link from README.md to (latest) tagged docs.
  • (Eventually) a documentation website.
@hiddeco
Copy link
Member Author

hiddeco commented Jul 4, 2023

Paging @onedr0p @devstein @sabre1041 (plus maybe @felixfontein).

@hiddeco hiddeco changed the title Simplify branching strategy and Simplify branching and documentation strategy Jul 4, 2023
@felixfontein
Copy link
Contributor

I really like switching the default branch to develop or a renamed version of it (I'm all in favor for main). Having the default branch of the repository different from the development branch caused quite a few mis-targeted PRs in the past, causing unnecessary work for reviewers (tell contributors to retarget the PR) and contributors (need to rebase/retarget).

I have no idea whether this change has an effect on other projects using sops as a library though. They might depend on git clone resulting in the latest released version, as opposed in the latest development version.

@onedr0p
Copy link
Contributor

onedr0p commented Jul 5, 2023

I kind of brought this up last year in #1013 so I'm down with these changes which are even an improvement on my comment in that issue.

@hiddeco
Copy link
Member Author

hiddeco commented Jul 5, 2023

A couple of things to take into account:

  1. When develop is moved to main, it just moves "ahead" and no real history is lost.

  2. For things which rely on e.g. master in GitHub URLs, they automatically detect the default branch as an alias for master. See for example: https://raw.githubusercontent.com/fluxcd/source-controller/master/main.go (this repository does not have a master branch).

  3. There will be some awkwardness around the logic in

    func RetrieveLatestVersionFromUpstream() (string, error) {

    I am personally not a big fan of the way this is written, and would rather like it to see based on e.g. Git tags and/or GitHub releases. While the actual Version would be injected during build (e.g. -ldflags="-X 'version.Version=<detected tag|detected Git commit>').

    This would theoretically be a breaking change, but arguably for the better (especially since this bit of logic being ~7 years old). Also note there have been request for an "offline" check as well (Do not report version when upstream check fails #1124).

@devstein
Copy link
Contributor

devstein commented Jul 5, 2023

@hiddeco +1 on the proposal. Is there a standard documentation website/framework within the CNCF?

I am personally not a big fan of the way this is written, and would rather like it to see based on e.g. Git tags and/or GitHub releases. While the actual Version would be injected during build (e.g. -ldflags="-X 'version.Version=<detected tag|detected Git commit>').

Same. This is what we do for KSOPS

@felixfontein
Copy link
Contributor

I'm also not a fan of that. That's also a reason why there's now a --disable-version-check option for sops --version - unfortunately it hasn't been released yet...

@hiddeco
Copy link
Member Author

hiddeco commented Jul 6, 2023

Is there a standard documentation website/framework within the CNCF?

There isn't, but you will commonly see:

With Flux, we started with Mkdocs but then moved to Hugo because we had desires for different page types than just documentation, and at the time it didn't have the blog functionality it has now (albeit sponsorware). I do however miss the simplicity of it these days.


Given we seem to have kind of reached consensus about the change, do we want to aim for making this change before we push out the next release? The bit around the --version command is a non-issue for the change itself, as even with develop becoming main it stays pinned at the last tagged release as long as we do not make changes to the build logic (and bump it in a release preparation PR).

@devstein
Copy link
Contributor

devstein commented Jul 6, 2023

@hiddeco I think we should try to make it part of the next release, but I'll defer to you. I know there are lot of moving parts for the next release as is.

@hiddeco
Copy link
Member Author

hiddeco commented Jul 6, 2023

  1. develop is now main.
  2. master has been removed after changing the base branch of outstanding PRs from master to main.
  3. Confirmed the URL from RetrieveLatestVersionFromUpstream to still be reachable as expected.

@onedr0p
Copy link
Contributor

onedr0p commented Jul 6, 2023

It's pretty neat github even notifies you of this change when you visit the repo. Someone at github knows UX 😄

@hiddeco
Copy link
Member Author

hiddeco commented Jul 6, 2023

There is one thing to note about this: you need to do the rename through the GitHub UI for this to happen. For getsops/sotp I did it through Git itself, and then the magic doesn't happen 😬

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

No branches or pull requests

4 participants