Skip to content

Put product version with git commit into model.zip/version.txt #3173

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

Merged

Conversation

Ivanidzo4ka
Copy link
Contributor

Fixes #3132

@codecov
Copy link

codecov bot commented Apr 2, 2019

Codecov Report

Merging #3173 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3173      +/-   ##
==========================================
- Coverage   72.54%   72.54%   -0.01%     
==========================================
  Files         807      807              
  Lines      144774   144775       +1     
  Branches    16208    16208              
==========================================
- Hits       105022   105021       -1     
- Misses      35338    35340       +2     
  Partials     4414     4414
Flag Coverage Δ
#Debug 72.54% <100%> (-0.01%) ⬇️
#production 68.13% <100%> (ø) ⬆️
#test 88.82% <100%> (-0.01%) ⬇️
Impacted Files Coverage Δ
test/Microsoft.ML.Functional.Tests/ModelFiles.cs 96.07% <100%> (ø) ⬆️
src/Microsoft.ML.Core/Data/Repository.cs 80.41% <100%> (+0.06%) ⬆️
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs 73.7% <0%> (-0.34%) ⬇️
...ML.Transforms/Text/StopWordsRemovingTransformer.cs 86.1% <0%> (-0.16%) ⬇️
...soft.ML.Data/DataLoadSave/Text/TextLoaderCursor.cs 84.9% <0%> (+0.2%) ⬆️
...StandardTrainers/Standard/LinearModelParameters.cs 60.31% <0%> (+0.26%) ⬆️

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

Thanks!

Do we want to take this for 1.0?

@Ivanidzo4ka
Copy link
Contributor Author

Ivanidzo4ka commented Apr 5, 2019

Do we want to take this for 1.0?

Well, in 1.0 version file would have 1.0.0.0 version in it, so I don't think it's super critical. For 1.1 or whatever future branch would be ,it would have this change, so, i'm not sure it's worth 1.0.
By I don't mind create PR against 1.0 release.

@eerhardt
Copy link
Member

eerhardt commented Apr 6, 2019

If/when we service the release/1.0 branch with 1.0.1 or 1.0.2, could we tell the difference? Or even if someone made a .zip file using a local build, instead of our released build.

@Ivanidzo4ka
Copy link
Contributor Author

Sure. I'll create PR towards 1.0 release. But first I think I need approvals for this one. @artidoro @abgoswam @wschin any one of you can take a look?

@Ivanidzo4ka Ivanidzo4ka merged commit 37ed336 into dotnet:master Apr 12, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 23, 2022
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.

Version.txt in model .zip should use the FileVersion, not AssemblyVersion
3 participants