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

[Feature] Same version computed on different branches #3898

Conversation

HHobeck
Copy link
Contributor

@HHobeck HHobeck commented Feb 6, 2024

[Feature] Same version computed on different branches:

Close #3453

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@HHobeck HHobeck changed the title [Feature] Same version computed on different branches #3453 [Feature] Same version computed on different branches Feb 6, 2024
@HHobeck HHobeck requested a review from asbjornu February 6, 2024 10:35
@HHobeck
Copy link
Contributor Author

HHobeck commented Feb 6, 2024

Hi @asbjornu. I'm not sure if you are agree with the new branch related property with name take-incremented-version. Please take a look and give me feedback. (#3455 (comment))

Thank's

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch from 799b9e2 to 5d0784b Compare February 6, 2024 12:25
@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch from 5d0784b to ad400b7 Compare February 7, 2024 14:00
@asbjornu
Copy link
Member

asbjornu commented Feb 8, 2024

Do I understand this issue correctly when it's about whether we want to consider the git tag of the current commit or not? If so, perhaps ignore-tag: False would be more intuitive?

@HHobeck
Copy link
Contributor Author

HHobeck commented Feb 9, 2024

Do I understand this issue correctly when it's about whether we want to consider the git tag of the current commit or not? If so, perhaps ignore-tag: False would be more intuitive?

At the end the criteria you are mentioned is correct. But you need to think about the semantic meaning. We have one or more strategies who are determining the existing base versions with an indication should the version be incremented yes or no. We are calculating based on this the next version and take the one which is highest. The result is we have exactly one base version and one incremented version (which might be equal). Now this feature comes in place where we want for some branches not to use the incremented version but the base version.

I can think of the following use cases:

  • Take always the base version and not the incremented version,
  • Take the base version when the current commit was released otherwise the incremented version,
  • Take always the incremented version and not the base version

Your proposed property with name ignore-tag does not reflect this semantic meaning if you ask me. It's also not about ignoring tags because the current tag will be considered and used to calculate the incremented version. If someone uses the ignore-tag property he could expect that all tags are ignored on the branch and not considered for the version calculation (like you would specify to ignore commits).

Does it make sense?

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch from ad400b7 to f3654ad Compare February 9, 2024 08:02
@asbjornu
Copy link
Member

asbjornu commented Feb 9, 2024

Right. But wouldn't then increment: None work, since for the specific branch we don't want to increment either Major, Minor or Patch?

@HHobeck
Copy link
Contributor Author

HHobeck commented Feb 9, 2024

Right. But wouldn't then increment: None work, since for the specific branch we don't want to increment either Major, Minor or Patch?

But we want to increment the version on the branch but only if the commit was not released (marked not with a tag).

Edit: If you increment the version with value 1.0.0-1 with none the result is 1.0.0-2 on branch with empty label. Which is not what you want.

image

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch from a63e469 to c0673dc Compare February 9, 2024 15:23
@HHobeck
Copy link
Contributor Author

HHobeck commented Feb 11, 2024

Maybe the following enum values are better:

internal enum TakeIncrementedVersion
{
    Never,
    IfNotReleased/Tagged,
    Always
}

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch 2 times, most recently from a985f4e to 7c10519 Compare February 15, 2024 21:36
@arturcic arturcic self-requested a review February 16, 2024 03:48
@HHobeck
Copy link
Contributor Author

HHobeck commented Feb 16, 2024

prevent-increment:
    when-versioned-branch-merged: true
    when-tagged: true

Actually I would like to do this in a separate PR if you not mind. Please review the changes and merge it to main. Thank you.

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch 5 times, most recently from 49eee78 to 847125e Compare February 18, 2024 19:15
@HHobeck HHobeck requested a review from asbjornu February 19, 2024 11:44
@arturcic
Copy link
Member

@HHobeck @asbjornu I'm done with the review, included the new schema changes.

@asbjornu when you have the time please have the review as well and then we can get it to the point to merge it.

@arturcic
Copy link
Member

@HHobeck please update your branch then rebase on top of origin/main

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch from e7b870b to 8b968a0 Compare February 19, 2024 16:47
@HHobeck
Copy link
Contributor Author

HHobeck commented Feb 22, 2024

I think this PR can be merged right!?

@arturcic
Copy link
Member

@HHobeck I think @asbjornu said he will try to review it this weekend.
I'd say we merge it on Monday if @asbjornu doesn't manage to review.

@HHobeck HHobeck force-pushed the feature/Same-version-computed-on-different-branches branch from 248ae66 to 589e353 Compare February 22, 2024 15:34
@arturcic
Copy link
Member

@asbjornu is anything left to review or we can merge?

@asbjornu asbjornu merged commit 45515e7 into GitTools:main Feb 22, 2024
138 checks passed
Copy link
Contributor

mergify bot commented Feb 22, 2024

Thank you @HHobeck for your contribution!

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

Successfully merging this pull request may close these issues.

[Feature] Same version computed on different branches
3 participants