fixes #101 fixes #336

[dazinator]

• Committed @MacDennis76 's fix which allows dynamic repository to run.. Although have no idea if this is an acceptable fix.

[gep13]

@andreasohlund @SimonCropp @distantcam

Can you guys help with the build machine:

This PR failing the build seems to have nothing to do with the actual PR changes.

[gep13]

@dazinator can you walk me through what tests you did to verify this change? Just want to be sure that we are on the same page, and if we can add some Unit/Integration Tests to cover this.

[dazinator]

Hi @gep13

Certainly!

First I verified that there is an issue by simply running GitVersion.exe with the debugger attached, passing in the necessary arguments, something like /url /u and /p which relate to the repo url and github username and password. I stepped through the code until it blew up. The exception was to do with the fact that the repo object doesn't have a "working directory" and the class responsible for finding the NextVersion.txt file, uses the working directory of the repo - which was null - causing an exception.

I commented on #101 to that effect, and was prepared to leave it at that but then I noticed #336 which was reporting the same problem. More specifically, I noticed @MacDennis76 suggestion which was this one liner code change.

As I didn't see a PR for this change, I made this change locally, ran GitVersion.exe with the debugger attached, and verified that it allows the program to run without throwing an exception.

However, I must stress, I have no idea whether the correctness of the program is in tact, or more importantly the correctness of this feature. There didn't appear to be any tests for it that I could find, otherwise I would have borrowed from those original tests to create a regression test.

I think getting some test coverage around this is much needed!

I hope that helps :)

[JakeGinnivan]

I think #350 may fix this issue.

Can you give it a go

git remote add jake [email protected]:JakeGinnivan/GitVersion.git
git fetch jake
git checkout jake/pr/dynamic -t

@GeertvanHorrik did most of the hard work, and it has a test now covering the dynamic repos feature.

[SimonCropp]

@gep13 ok the disk space issue is weird. No idea why it would fail and then succeed immediately after. Perhaps @gbiellem has an idea?

Apart from that I say we call it a "ghost in the machine" and see if it re-appears

[gep13]

Apart from that I say we call it a "ghost in the machine" and see if it re-appears

Sounds good to me 👍

[gbiellem]

I took a look at thet build agent... Plenty of disk space is available ( in excess of 5 GB) so this is very weird...

[gep13]

I took a look at thet build agent... Plenty of disk space is available ( in excess of 5 GB) so this is very weird...

Very strange indeed!

[dazinator]

There's a ghost in my PR! :)
By the way pretty sure this PR is now superceded anyway by #350 as @JakeGinnivan mentioned. I am about to give #350 a go now. If it works, i'll close this PR as it shouldn't be needed :)

[dazinator]

Superceded by #350 - closing