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

[Bug] ignore parameter does not ignore everything #3074

Closed
ibvhefe opened this issue Apr 10, 2022 · 10 comments · Fixed by #3953
Closed

[Bug] ignore parameter does not ignore everything #3074

ibvhefe opened this issue Apr 10, 2022 · 10 comments · Fixed by #3953
Labels
Milestone

Comments

@ibvhefe
Copy link

ibvhefe commented Apr 10, 2022

Describe the bug

  1. Checkout the support branch of this project (Gitversion/support-branch)
  2. Change the GitVersion.yml to
mode: Mainline
branches: {}
ignore:
  sha: [ ]
  commits-before: 2023-04-10T01:00:00 #some date in the future
merge-message-formats: {}
  1. Run GitVersion

Expected Behavior

A version such as 0.1.0

Actual Behavior

The calculated version seems to be too high:

{
  "Major": 1,
  "Minor": 2,
  "Patch": 111
  ...
}
@ibvhefe ibvhefe added the bug label Apr 10, 2022
@asbjornu
Copy link
Member

Although I don't see the value of this, I agree that ignoring the entire repository should yield a 0.1.0 version number. Pull requests are welcome.

@rominator1983
Copy link
Contributor

I'm also in need of this and started a branch here https://github.com/rominator1983/GitVersion/tree/fix/IgnoreAllCommits_Issue3074 to make a PR

The value of this (at least that's what I hope) is to mitigate erroneous commits.
For example bit-bucket in its default configuration adds the latest 20 commit message of a source branch to a PR merge. This duplicates the "+semver:major" commit message which can't possibly be undone if you just see this some weeks later and there are several people on the project. 😦

I was wondering about the tests. I ran the tests 3 locally and got 21 (for tag 5.10.3) or 10 (main) red tests.
I was using Visual Studio 2022 and .net 6 as a runtime. I also installed .net core 3.1 since Visual Studio told me to do so. This unfortunately did not make any difference.

Here is an example of a broken test. NextVersionCalculatorTests.PreReleaseNumberShouldBeScopeToPreReleaseLabelInContinuousDelivery:

   Source: NextVersionCalculatorTests.cs line 279
   Duration: 1,1 sec

  Message: 
Shouldly.ShouldAssertException : variables.FullSemVer
    should be
"0.1.0-beta.1+2"
    but was
"0.1.0-beta.1+3"
    difference
Difference     |                                                                   |   
               |                                                                  \|/  
Index          | 0    1    2    3    4    5    6    7    8    9    10   11   12   13   
Expected Value | 0    .    1    .    0    -    b    e    t    a    .    1    +    2    
Actual Value   | 0    .    1    .    0    -    b    e    t    a    .    1    +    3    
Expected Code  | 48   46   49   46   48   45   98   101  116  97   46   49   43   50   
Actual Code    | 48   46   49   46   48   45   98   101  116  97   46   49   43   51   

  Stack Trace: 
GitToolsTestingExtensions.AssertFullSemver(RepositoryFixtureBase fixture, String fullSemver, Config configuration, IRepository repository, String commitId, Boolean onlyTrackedBranches, String targetBranch) line 116
NextVersionCalculatorTests.PreReleaseNumberShouldBeScopeToPreReleaseLabelInContinuousDelivery() line 308

Also I tried to run a documentation build as described in docs/readme.md but the command .\build.ps1 -Target Preview-Documentation gives me Error: The target 'Preview-Documentation' was not found.

So this makes it difficult for me to tell if my changes are fine or if existing tests need to be adapted because of a changed behavior.

@rominator1983
Copy link
Contributor

@ibvhefe you think you could try if the PR fixes your issue?

@ibvhefe
Copy link
Author

ibvhefe commented Aug 26, 2022

@rominator1983 thanks a lot for caring and providing a fix. Unfortunately I am on vacation and do not have access to a computer. I can try it out in two weeks. Sorry

@github-actions
Copy link

github-actions bot commented Mar 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Mar 7, 2023
@HHobeck HHobeck removed the stale label Apr 1, 2023
@HHobeck HHobeck added this to the 6.x milestone Apr 1, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Jun 30, 2023
@arturcic arturcic added stale and removed stale labels Jun 30, 2023
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Sep 29, 2023
@arturcic arturcic removed the stale label Sep 29, 2023
@arturcic arturcic added stale and removed stale labels Oct 30, 2023
@HHobeck HHobeck modified the milestones: 6.x, 7.x Jan 11, 2024
@HHobeck HHobeck modified the milestones: 7.x, 6.x Feb 18, 2024
@HHobeck
Copy link
Contributor

HHobeck commented Mar 13, 2024

Actually if you specify some date in the future the behaviour of GitVersion should be throwing an exception because it is equivalent to the case you have an empty repository. GitVersion needs at least one commit to determine a semantic version.

@asbjornu: Are you agree?

Attachment[1]:
image

@asbjornu
Copy link
Member

@HHobeck, I think it's debatable whether GitVersion should always try to produce a version number, even in obvious invalid circumstances (such as the repository having no reachable commits, missing .git directory, etc.). Perhaps it can be the default modus operandi of GitVersion in v7 or something, when we have a new CLI and can add a flag like --treat-warnings-as-errors? I at least like the idea of always writing JSON output to stdout so GitVersion's integration with CI/CD pipelines becomes as friction-free as possible.

@arturcic
Copy link
Member

🎉 This issue has been resolved in version 6.0.0-beta.7 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

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