search

getsops / sops

Simple and flexible tool for managing secrets
https://getsops.io/
Mozilla Public License 2.0
17.1k stars 879 forks source link

Simplify branching and documentation strategy #1238

Open hiddeco opened 1 year ago

hiddeco commented 1 year ago

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 (https://github.com/getsops/community/issues/9), while also renaming master to main (https://github.com/getsops/community/issues/16).

Proposed alternative

hiddeco commented 1 year ago

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

felixfontein commented 1 year ago

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 commented 1 year ago

I kind of brought this up last year in https://github.com/getsops/sops/issues/1013 so I'm down with these changes which are even an improvement on my comment in that issue.

hiddeco commented 1 year ago

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 https://github.com/getsops/sops/blob/e1edc059487ddd14236dfe47267b05052f6c20b4/version/version.go#L60

    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 (#1124).

devstein commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago
  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 commented 1 year ago

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

hiddeco commented 1 year ago

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 :grimacing: