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

[Docs]: Confusing/missing documentation around strategies #4219

Open
2 tasks done
selfdocumentingcode opened this issue Sep 20, 2024 · 5 comments
Open
2 tasks done

[Docs]: Confusing/missing documentation around strategies #4219

selfdocumentingcode opened this issue Sep 20, 2024 · 5 comments
Milestone

Comments

@selfdocumentingcode
Copy link

selfdocumentingcode commented Sep 20, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched issues to ensure it has not already been reported

GitVersion package

GitVersion.Tool

GitVersion version

6.x

Operating system

Linux

What are you seeing?

https://gitversion.net/docs/learn/how-it-works lists:

  • HighestReachableTag
  • NextVersionInConfig
  • MergedBranchWithVersion
  • HighestTagBaseVersionStrategy
  • VersionInBranchBaseVersionStrategy
  • ConfigNextVersionBaseVersionStrategy
  • MergeMessageBaseVersionStrategy
  • FallbackBaseVersionStrategy

Issues:

  • There's no HighestReachableTag strategy
  • There's no HighestTagBaseVersionStrategy strategy
  • NextVersionInConfig should be called ConfigureNextVersionVersionStrategy
  • ConfigNextVersionBaseVersionStrategy should be called ConfigureNextVersionVersionStrategy
  • TrackReleaseBranchesVersionStrategy is not documented
  • MainlineVersionStrategy is not documented
  • There's nothing called "BaseVersionStrategy" in the code (sans a few tests classes) or other places in the documentation.

https://gitversion.net/docs/reference/configuration lists:

  • TrunkBased

Issues:

  • Should list Mainline instead

What is expected?

Versioning strategies to be correctly documented.

Steps to Reproduce

Read documentation

RepositoryFixture Test

No response

Output log or link to your CI build (if appropriate).

No response

@HHobeck
Copy link
Contributor

HHobeck commented Sep 20, 2024

Yes you are right, there is a mismatch between the code and the documentation. I know at some point we discussed that the naming doesn't reflect the intention of each version strategy. But we have skipped this discussion. Maybe now is the time to rename it if necessary. Personally, I have no preferences.

Here is my view:

https://gitversion.net/docs/learn/how-it-works lists:

HighestReachableTag -->>TaggedCommitVersionStrategy
NextVersionInConfig -->>ConfiguredNextVersionVersionStrategy
MergedBranchWithVersion -->>MergeMessageVersionStrategy
HighestTagBaseVersionStrategy -->>TaggedCommitVersionStrategy
VersionInBranchBaseVersionStrategy -->>TrackReleaseBranchesVersionStrategy
ConfigNextVersionBaseVersionStrategy -->>ConfiguredNextVersionVersionStrategy
MergeMessageBaseVersionStrategy -->>TrackReleaseBranchesVersionStrategy
FallbackBaseVersionStrategy -->>FallbacktVersionStrategy
Issues:

There's no HighestReachableTag strategy
There's no HighestTagBaseVersionStrategy strategy
NextVersionInConfig should be called ConfigureNextVersionVersionStrategy
ConfigNextVersionBaseVersionStrategy should be called ConfigureNextVersionVersionStrategy
TrackReleaseBranchesVersionStrategy is not documented
MainlineVersionStrategy is not documented -->>It can be seen like the previous Mainline deployment mode (which was removed and implemented in Mainline version strategy). Because the unit tests are still existing and green, this feature is still there. Probably we need to move the documentation about Mainline deployment mode to this section you mentioned it is missing. ;)
There's nothing called "BaseVersionStrategy" in the code (sans a few tests classes) or other places in the documentation. -->>The base in BaseVersionStrategy was eliminated
https://gitversion.net/docs/reference/configuration lists:

TrunkBased
Issues:

Should list Mainline instead -->> I agree

@selfdocumentingcode
Copy link
Author

Thanks for reviewing the issue.

From what I've managed to gather from the code, it looks to me that TrackReleaseBranchesVersionStrategy implementation is just not in use.

I thought it might have something to do with the TrackReleaseBranches property for branch configurations, but that property is only referenced in the MainlineVersionStrategy implementation.

I created a PR attempting to fix some of these documentation issues here #4220

@HHobeck
Copy link
Contributor

HHobeck commented Sep 23, 2024

Thanks for reviewing the issue.

From what I've managed to gather from the code, it looks to me that TrackReleaseBranchesVersionStrategy implementation is just not in use.

Thank you taking the time to correct the documentation. What do you mean with the TrackReleaseBranchesVersionStrategy implementation is not in use? All version strategy classes are implementing the IVersionStrategy interface, which is used to detect the algorithm via reflection.

image

I thought it might have something to do with the TrackReleaseBranches property for branch configurations, but that property is only referenced in the MainlineVersionStrategy implementation.

Please notice the branch related property with name TracksReleaseBranches is used in the class TrackReleaseBranchesVersionStrategy as well.

@selfdocumentingcode
Copy link
Author

What do you mean with the TrackReleaseBranchesVersionStrategy implementation is not in use? All version strategy classes are implementing the IVersionStrategy interface, which is used to detect the algorithm via reflection.

Oh! It did cross my mind that loading it via reflection might be the case, but I somehow missed it when I searched the codebase.
Thanks for pointing it out to me!

@selfdocumentingcode
Copy link
Author

Please notice the branch related property with name TracksReleaseBranches is used in the class TrackReleaseBranchesVersionStrategy as well.

I see now. I was only looking at the references to the TracksReleaseBranches property from IBranchConfiguration and not to the one in EffectiveConfiguration.
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants