MRTK Master Release Packages #1530
Conversation
|
These new packages don't follow the same naming standard as the previous packages, so it appears out of order. Should it not be: Or should the Version number be increased as well? |
|
We decided to drop the v1 |
|
These file names match the release tag version |
|
Righteo, just testing tonight (while waiting on other comments) |
|
Best to not merge this until #1471 is done and then update. To ensure it has the latest Build Window |
|
1471 didn't make it into the release I made these packages for. It'll likely be in the next release. |
| // Defaulting coordinate system to Stationary for transparent headsets, like HoloLens. | ||
| // This puts the origin (0, 0, 0) at the first place where the user started the application. | ||
| private TrackingSpaceType transparentTrackingSpaceType = TrackingSpaceType.Stationary; | ||
| //private TrackingSpaceType transparentTrackingSpaceType = TrackingSpaceType.Stationary; |
There was a problem hiding this comment.
The non-package parts of this PR look like they should be added to the dev branch instead of into master.
There was a problem hiding this comment.
Well I already added these changes to the packages and they're already released, soo....
There was a problem hiding this comment.
The packages attached to the release aren't actually a snapshot of master...? That seems counter to the entire idea of a release.
Those packages can also be updated to the correct ones, as they're a GitHub construct and not a git one.
There was a problem hiding this comment.
Well the stabilization branch got merged into master before I could review it, and this was the stuff that needed to be fixed. I don't like making releases that include warnings or issues building, which is what the changes outlined show.
So you'd rather put out a broken release?
There was a problem hiding this comment.
Packages need to be from master. Is that the case?
There was a problem hiding this comment.
They are. With the fixes addressed.
|
I wasn't about to publish a release with known bugs that were easy to address, So I fixed them. In the future I'd like to have the time to review before making merges back into the master branch. |
|
@StephenHodgson we have already marked terrible releases :) Sample the one before the holiday release. I agree in spirit. But your fixes just seem warnings. Far less benign that older ones which did not even do basics correctly. Why dont you craft a patch holiday release with your fixes if you feel so passionate about these? We already have that avenue :) Mismatching packages are just lying about the source truth. |
|
Not a warning if you can't build for a specific platform ;) |
Which is what if PR aims to address and fix |
|
After Shiproom meeting it was decided to update the released packages and make a patch release. |
Added Release packages and removed one compiler warning