camaraproject / QualityOnDemand

Repository to describe, develop, document and test the QualityOnDemand API family
https://wiki.camaraproject.org/x/zwOeAQ
Apache License 2.0
41 stars 59 forks source link

Fixed couple of typos #77

Closed sfnuser closed 1 year ago

sfnuser commented 1 year ago

Fixed couple of typos

hdamker commented 1 year ago

Also fine with the obvious fixes. Just thinking about the appropriate version (number) to target.

Two options: a) fix v0.8.0 to v0.8.1 (a bugfix, not sure if there could be client implementations break, the minor version would just allow to see that something changed) b) correct it only in v0.9.0 (so target the PR new to dev-v0.9.0 branch)

@eric-murray @jlurien what are your thoughts?

eric-murray commented 1 year ago

My preference would be that approved PRs get their own version number, rather than "rolling up" multiple PRs into a larger update. So this should be v0.8.1 (as it is documentation changes and should not be breaking any client implementations).

Of course, this begs the question as to what would happen if another PR that itself required a minor version increment was raised and approved before the dev-v0.9.0 branch was approved. I'd suggest that branches should be named after the feature they are implementing / fixing rather than the expected target version number, and the version number only assigned when the PR is merged.

But this problem could also be avoided by exhaustively discussing proposed changes in a related Github issue first before raising the PR, rather than raising the PR prematurely and then debating it for months. In that case, approving the PR should be a formality, and the required version number then known in advance.

jlurien commented 1 year ago

This v0.8 is special as it is a milestone for MWC. However I think it is OK to correct these detected typos, and if we change anything in yaml updating to 0.8.1 is good for tracking. We may think also in having some changelog.md to document the changes between versions, and it may be useful to have as well all past versions in a history folder. Especially, the v0.8.Z version should be easily accesible when we merge a future version.

Regarding future versions and branches, my understanding is that now we have a dev-vX.Y branch for next relevant version where all new PRs are proposed until we decide that we can consolidate that branch as a new version and merge it to main. Any unresolved PRs in this dev- branch should then be moved to the a new dev branch for subsequent version.

Independently to this I agree that it is good to open first an issue and get some initial consensus, although in the end a proposal is better understood and commented in a PR.

kaikreuzer commented 1 year ago

This discussion sounds to me as if there isn't yet a clear branching/versioning strategy (what is always a complex topic indeed, but one cannot escape it).

Many (if not most) Github projects consider the main branch to be the "latest" code that will eventually be the next release. Therefore, the main branch is the default against which PRs are automatically opened (as is the case for this repo as the setting has not been changed). Projects then rather branch off older releases (such as 0.8.x) to provide further patch releases from such branches, while continuing the latest development on main.

From a contributors perspective, the branch against which PRs are by default created (here: main) is the one where they should be merged. Contributors do not care about your version/release management - so if you merge it here for the future 0.9.0 release, this is fine and you are still free to decide to also cherry-pick it to an 0.8.x-branch, where you could decide to do a 0.8.1 patch release from.

I agree that it is good to open first an issue and get some initial consensus

For major features where it is not clear where they are heading, I agree. For PRs like this one at hand, your processes must allow to very quickly and easily merge it. The only discussion that could happen is the one about the decision to backport it for patch releases or not - but that can imho happen after a merge.