Skip to content
This repository has been archived by the owner on Nov 20, 2020. It is now read-only.

Pad build numbers and prefix PRs #9

Merged
merged 1 commit into from
Oct 12, 2018
Merged

Pad build numbers and prefix PRs #9

merged 1 commit into from
Oct 12, 2018

Conversation

gusty
Copy link
Contributor

@gusty gusty commented Oct 11, 2018

This will create stuff like:
MyLib.0.1.0-CI0000798.nupkg (from master)
MyLib.0.1.0-pr17-0000798.nupkg (from PRs)

This will create stuff like:
`MyLib.0.1.0-CI0000798.nupkg` (from master)
`MyLib.0.1.0-pr17-0000798.nupkg` (from PRs)
SET X=%1
SET BLD=000000%X%
SET BLD=%BLD:~-7%
if [%2]==[] (SET V=CI%BLD%) else (SET V=pr%2-%BLD%)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is looking more and more like a job for a build.proj ;)

Copy link
Contributor

@bartelink bartelink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local build is still reporting

C:\code\CallPolly\src\CallPolly\obj\Release\netstandard2.0\CallPolly.AssemblyInfo.fs(15,69): warning FS2003: The attribute System.Reflection.AssemblyInformationalVersionAttribute specified version '0.0.2-CI000000', but this value is invalid and has been ignored [C:\code\CallPolly\src\CallPolly\CallPolly.fsproj]

Feel free to merge, but my dream is to have a build.proj with no warnings
also this needs to happen in the upstream Jet.JsonNet.Converters too

@gusty
Copy link
Contributor Author

gusty commented Oct 12, 2018

That's an F# bug, apparently it's already fixed but still not shipped: dotnet/fsharp#4822

@bartelink
Copy link
Contributor

Ah, good to know - so, do you want to port it to a build.proj are are we going to :shipit: ?
(I'll copy whatever you decide to merge into the converters project)
NB I know the great is the enemy of the good, but I've been suggesting to others to clone from one of these as a way to get a good template-project; if we don't polish this sort of stuff now, we'll have ripples of manual work to fix this when we get to this (I'm guessing someone wanting to build this on non-Windows will end up doing it... - except they'll probably port this to an even worse .sh monstrosity)

@gusty
Copy link
Contributor Author

gusty commented Oct 12, 2018

Merging this for now. I'll keep improving the build infrastructure in separate PRs.

@gusty gusty merged commit 31dc39d into master Oct 12, 2018
@bartelink
Copy link
Contributor

I shaved this off master as it failed there (and when I put my thing on top).
Not unrelated ot 👆 is this nice article on doing a build.proj version of all this

@gusty
Copy link
Contributor Author

gusty commented Oct 14, 2018

What's the failure?

I have a build.proj already. See the one in XRay.

@bartelink
Copy link
Contributor

About pull request id - see the master commit log. Unfortunately the removal of your commit did not help as the build config cant cope with that (we also need to fix that - I'll leave it the way it was now)

@bartelink
Copy link
Contributor

or is our build numbering predicated on the commit index on a branch? (for early days of a project I tend to do force pushes to keep history neat rather than doing formal revert PRs and tiny fixup PRs)

@bartelink
Copy link
Contributor

@gusty
Copy link
Contributor Author

gusty commented Oct 14, 2018

Weird, I'll have a look shortly, but it seems to be a problem in the pipelines rather than the build script.

That's why removing this commit didn't solve it.

@bartelink
Copy link
Contributor

You sure ? the error on my commit and your commit was similar about build numbers.

When I pushed without it, it was a detached head error - i.e. it did not get as far as runningth build.cmd

@gusty gusty mentioned this pull request Oct 14, 2018
@gusty
Copy link
Contributor Author

gusty commented Oct 14, 2018

The problem was that Azure Pipelines was not expanding uninitialized variables.
I did a workaround in the Pipeline itself, so now it should work fine for both projects.

@bartelink
Copy link
Contributor

OK, I've elided it from master to save the distration of having to analyze - #12 will presumably reinstate when it merges (I'm doing some integration work now in #11 which I will merge as soon as)

@bartelink bartelink deleted the gusty-patch-build branch October 15, 2018 14:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants