Can't receive token in release version

Hi! Thanks for great plugin!
My application is configured by your guide and all works like a charm, but only when I launch app in debug mode
XCode - product - scheme - edit scheme - run - build configuration - debug

on('registration', ...) callback successfully fired, I receive token and can send to it notifications and receive them on device
When I launch in release mode - on('registration', ...) callback doesn't fire at all. Also on('error', ...) callback also not fired and device logs are empty.
Maybe you know what could be the problem?
Thanks in advance.

I am having the same exact problem. I've tried with regular distribution and enterprise distribution but I never get a token. Works perfectly in debug mode. Don't get any error handlers firing either.

Hi Hitman666, I know this is a free plugin you created so not a lot of incentive to make sure it's update to date/always working, but it'd really appreciate if you could take a look, I'm thinking it's something small as it does work just not in release mode.  I'm actually updating an app that had this plugin working correctly with it (1.0.1), but I think maybe an iOS11 change did something, because if I try that version as well, it's no longer working.

Hey guys,
I'm just acknowledging that I do read these and that I'm going to check. I can't promise that this will be before next weekend though :/

@Hitman666 thanks for answer. Maybe it will be helpful. I tested phonegap-plugin-push that based on your plugin and it work in release mode. Probably you can find fixes for your module there

@Hitman666 thanks for lettings us know. Really appreciate the work you do!

it would be great if this could be fixed!

@EugeneSnihovsky We are using phonegap-plugin-push for normal Push Notifications. Is it possible to use both VoIP push + normal notifications at the same time with phonegap-plugin-push ?

@AleksandarTokarev
Unfortunatelly I don't know answer for your question.
It took a long time and I don't remember the details, but in that project I completely abandoned the cordova-ios-voip-push and use phonegap-plugin-push to receive VoIP notifications.
When I finish project in 2018 I stopped working with ionic/cordova and forgot them like a bad dream.
If you're still at the prototyping stage I recommend you switch to react-native. The ionic/cordova comunity is dead. Especially with regard to native functionality. You will have to solve all arising problems yourself


erlang/otp 20 , pooler 1.5.2, can not compile

==> "pooler"
Compiling /deps/pooler/src/pooler.erl
Line 20: error {undefined_type,{dict,0}} in "/deps/pooler/src/pooler.hrl"
Line 21: error {undefined_type,{queue,0}} in "/deps/pooler/src/pooler.hrl"
ERROR: []
escript: exception error: no function clause matching
lists:flatten({return,{return,[{error,[]}]}}) (lists.erl, line 616)

it seems the compatibility problem is not with erlang but with https://github.com/synrc/mad
https://github.com/silviucpp/mysql_pool/issues/3

https://github.com/221V/pooler/commits/master
this fix works
mad is ok


Fixed crf_log_norm function in crf.py

The original version of crf_log_norm function will not output zero when the sequence length is zero.
This patch fixed the problem.

Can one of the admins verify this patch?


Add images directory to project

Hi 👋🏻
Please add a directory for images in this project.
Regards!

This commit bf2c5c1c8ec2a02c3fc9d486091d335a6b436fa6 has resolved this issue


New package: EntropyInfo v1.0.0-DEV

Registering package: EntropyInfo
Repository: https://github.com/EntropyInfo/EntropyInfo.jl
Created by: @EntropyInfo
Version: v1.0.0-DEV
Commit: 308d8c15a2b5322bfe38fd0a644500c5774bf401
Git reference: HEAD
Release notes:

First release.
Inv + KNN + Histogram of entropy base metrics.

Hi, congrats on the package!
Can I suggest picking a slightly more specific name? From what I understand, your package provides one specific method for several variants of entropy. Maybe you could name it after that method?
Also I wasn't able to find the paper you cite

An invariant estimation of entropy and mutual information

Hi Thanks,
I have change the name to EntropyInvariant.
The paper is not yet published.
We can close this pullrequest.

Closing in favor of https://github.com/JuliaRegistries/General/pull/120279


[ADMIN] FHL scrub

Docs Build status updates of commit c59b935:
:x: Validation status: errors
Please follow instructions here which may help to resolve issue.

File
Status
Preview URL
Details

:x:Error

Details

[Error]

Cannot sync git repo to specified commit because branch lindalu-FHL-scrub-day3 has been deleted or has been force pushed: fatal: couldn't find remote ref refs/heads/lindalu-FHL-scrub-day3

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


Specify total libyear limit

As a user, I want to know when an arbitrary age limit is exceeded, so that I can take action on my decrepit app.
Acceptance criteria:

new --limit/-l cli arg returns exit code 1 if total libyears behind is greater than limit

Implemented in the just-released v7!


Add content about how to work with dynamic multiple choice and arrays

Related:
https://github.com/microsoft/BotFramework-Composer/issues/2232

I go round in a loop if I follow your comment/link

I go round in a loop if I follow your comment/link

@philip-mc I reproduced your code and got the exact issue you had. My guess is that when we initialize an array that contains multiple objects, we might need specific settings in the multi-choice prompt to make it work.
While still investigating your problem, I'd like to share some basic steps to use a dynamic multi-choice prompt in Composer. P.S. We used to have a "dynamic" setting which was removed from Composer for some reason. These instructions are based on what I experimented with from my end and the comments shared by @hibrenda on this original issue.

Let's set a property and assign an array of strings as its property value.
Then we create a multi-choice prompt.

Enter the text you would like to prompt in the Bot Asks seciton.
In the User Input section, set List style to auto and set the value of Array of choices to =thePropertyNameSetInStep1

The following are the code for your reference:

Testing in Emulator:

@philip-mc I reproduced your code and got the exact issue you had. My guess is that when we initialize an array that contains multiple objects, we might need specific settings in the multi-choice prompt to make it work.
While still investigating your problem, I'd like to share some basic steps to use a dynamic multi-choice prompt in Composer. P.S. We used to have a "dynamic" setting which was removed from Composer for some reason. These instructions are based on what I experimented with from my end and the comments shared by @hibrenda on this original issue.

Let's set a property and assign an array of strings as its property value.
Then we create a multi-choice prompt.

Enter the text you would like to prompt in the Bot Asks seciton.
In the User Input section, set List style to auto and set the value of Array of choices to =thePropertyNameSetInStep1

The following are the code for your reference:

Testing in Emulator:

Thank you so much for the assistance. It was the strucutre of the array that had it spluttering:

The expression was replaced with the lower one and it worked straight away. For the documentation can I recommend examples are added all the way along as these are the most valuable tool I have in getting dialogs working :D

Thank you so much for the assistance. It was the strucutre of the array that had it spluttering:

The expression was replaced with the lower one and it worked straight away. For the documentation can I recommend examples are added all the way along as these are the most valuable tool I have in getting dialogs working :D

This is good feedback. Thanks a lot! I will add some content to this article soon.
Thanks again.

This is good feedback. Thanks a lot! I will add some content to this article soon.
Thanks again.


x/playground: timeout compiling trivial code that imports net/http

If I try to run the example here:
https://golang.org/pkg/net/http/httputil#DumpResponse
I get this:
timeout running go build

Go build failed.

Same result if using same code on the playground:
https://play.golang.org/p/nf79uLXs5-b
If I run locally, it works fine:
"HTTP/1.1 200 OK\r\nContent-Length: 76\r\nContent-Type: text/plain;
charset=utf-8\r\nDate: Wed, 19 Jul 1972 19:00:00 GMT\r\n\r\nGo is a
general-purpose language designed with systems programming in mind.\n"

It seems to happen with an even simpler program that imports net/http for side-effects only. For example, I'm getting a "timeout running go build" when trying to run:
package main

import (
	_ "fmt"
	_ "net/http"
)

func main() {
	fmt.Println("output")
}

Instead of the expected build error like "./prog.go:8:2: undefined: fmt".
CC @golang/release.

It's possible that our changes in #44714 and #44389 caused longer compilation times due to differences in what is considered stale now. We might have to increase the compilation timeout to account for this.

closing for lack of interest

I am going to re-open this. I want to fix it, but have been busy with other things.

@89z FYI, the linked snippet does pass for me after our changes, but still occasionally fails: https://play.golang.org/p/nf79uLXs5-b
Modules by default impacted our caching strategy for dependencies on the playground, and we've not figured out the best way to resolve that yet.

It's possible that our changes in #44714 and #44389 caused longer compilation times due to differences in what is considered stale now. We might have to increase the compilation timeout to account for this.

closing for lack of interest

I closed this for a reason. That reason, hasnt changed. Please open a new issue, if you feel so inclined.

@89z seems like you identified a real problem which the team is putting real effort into fixing. Ongoing bugs should be tracked by open issues; therefore, I don't see why we should close this issue until the problem is fixed (which seems imminent). If you "lack interest" in this issue, you are welcome to just stop interacting with it.
Additionally, multiple merged and in-progress CLs already reference this issue number, so switching to a new issue now just makes the history harder to follow.

@cespare OK fair enough. I respect you because of your comments on Go generics, so whatever you say.

@89z You should be able to click on the "Unsubscribe" button over on the right if you no longer care about this issue.  Thanks.

The playground should deploy shortly, and I'll update here after verifying this was fixed.

It worked! Thanks for reporting @89z.


[BUG] System.AggregateException: One or more errors occurred. (The given key 'content-platform-dictionaries.zip' was not present in the dictionary.)  [Inner Exception] System.Collections.Generic.KeyNotFoundException: The given key 'content-platform-dictionaries.zip' was not present in the dictionary. while installing old bloxstrap

Acknowledgement of preliminary instructions

[X] I have read the preliminary instructions, and I am certain that my problem has not already been addressed.
[X] I have thoroughly looked through the available Wiki articles and could not find a solution to my problem.
[X] I am using the latest version of Bloxstrap.
[ ] I did not answer truthfully to all the above checkboxes.

Bloxstrap Version
2.5.4
What problem did you encounter?
since the 21 graphic bar
has been removed i wanted to try old bloxstrap but i wouldnt install it because this error

Bloxstrap Log
2024-12-07T10:37:23Z [App::OnStartup] Starting Bloxstrap v2.5.4
2024-12-07T10:37:23Z [App::OnStartup] Compiled Saturday, 4 November 2023 at 3:46:02 AM from commit 880c076fcd875fc10b5f6bef770033074d79a7a8 (tag/v2.5.4)
2024-12-07T10:37:23Z [App::OnStartup] Loaded from %UserProfile%\OneDrive\Desktop\Bloxstrap-v2.5.4.exe
2024-12-07T10:37:23Z [InstallChecker::FirstTimeRun] Running first-time install
2024-12-07T10:37:23Z [Logger::Initialize] Initializing at %UserProfile%\AppData\Local\Temp\Bloxstrap_20241207T103723Z.log
2024-12-07T10:37:23Z [Logger::Initialize] Finished initializing!
2024-12-07T10:37:23Z [JsonManager<Dictionary`2>::Load] Loading from ClientSettings\ClientAppSettings.json...
2024-12-07T10:37:23Z [JsonManager<Dictionary`2>::Load] Failed to load!
2024-12-07T10:37:23Z [JsonManager<Dictionary`2>::Load] Could not find a part of the path '%UserProfile%\OneDrive\Desktop\ClientSettings\ClientAppSettings.json'.
2024-12-07T10:37:23Z [FastFlagManager::SetValue] Setting of 'FFlagHandleAltEnterFullscreenManually' to 'False' is pending
2024-12-07T10:37:23Z [FastFlagManager::SetValue] Setting of 'FLogNetwork' to '7' is pending
2024-12-07T10:37:23Z [FastFlagManager::SetValue] Setting of 'DFIntTaskSchedulerTargetFps' to '120' is pending
2024-12-07T10:37:23Z [MainWindow::MainWindow] Initializing menu
2024-12-07T10:37:28Z [FastFlagManager::SetValue] Setting of 'FFlagFixGraphicsQuality' to 'True' is pending
2024-12-07T10:37:33Z [NotifyIconWrapper::NotifyIconWrapper] Initializing notification area icon
2024-12-07T10:37:33Z [App::OnStartup] Initializing bootstrapper
2024-12-07T10:37:33Z [App::OnStartup] Initializing bootstrapper dialog
2024-12-07T10:37:33Z [Bootstrapper::Run] Running bootstrapper
2024-12-07T10:37:33Z [Bootstrapper::Run] Performing connectivity check...
2024-12-07T10:37:33Z [Bootstrapper::SetStatus] Connecting to Roblox...
2024-12-07T10:37:33Z [RobloxDeployment::GetInfo] Getting deploy info for channel LIVE (extraInformation=False)
2024-12-07T10:37:33Z [HttpClientLoggingHandler::ProcessRequest] GET https://clientsettingscdn.roblox.com/v2/client-version/WindowsPlayer/channel/LIVE
2024-12-07T10:37:34Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://clientsettingscdn.roblox.com/v2/client-version/WindowsPlayer/channel/LIVE
2024-12-07T10:37:34Z [Bootstrapper::Run] Connectivity check finished
2024-12-07T10:37:34Z [RobloxDeployment::GetInfo] Getting deploy info for channel LIVE (extraInformation=False)
2024-12-07T10:37:34Z [RobloxDeployment::GetInfo] Deploy information is cached
2024-12-07T10:37:34Z [DeployManager::DefaultBaseUrl.Set] Testing connection to 'https://setup.rbxcdn.com'...
2024-12-07T10:37:34Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version
2024-12-07T10:37:35Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version
2024-12-07T10:37:35Z [DeployManager::DefaultBaseUrl.Set] Connection successful!
2024-12-07T10:37:35Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-rbxPkgManifest.txt
2024-12-07T10:37:36Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-rbxPkgManifest.txt
2024-12-07T10:37:36Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-rbxManifest.txt
2024-12-07T10:37:36Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-rbxManifest.txt
2024-12-07T10:37:36Z [Bootstrapper::SetStatus] Installing Roblox...
2024-12-07T10:37:36Z [Bootstrapper::DownloadPackage.RobloxApp.zip] Downloading...
2024-12-07T10:37:36Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-RobloxApp.zip
2024-12-07T10:37:36Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-RobloxApp.zip
2024-12-07T10:38:22Z [Bootstrapper::DownloadPackage.RobloxApp.zip] Finished downloading! (88797938 bytes total)
2024-12-07T10:38:22Z [Bootstrapper::ExtractPackage] Reading RobloxApp.zip...
2024-12-07T10:38:22Z [Bootstrapper::DownloadPackage.content-avatar.zip] Downloading...
2024-12-07T10:38:22Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-avatar.zip
2024-12-07T10:38:22Z [Bootstrapper::ExtractPackage] Read RobloxApp.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1...
2024-12-07T10:38:22Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-avatar.zip
2024-12-07T10:38:22Z [Bootstrapper::DownloadPackage.content-avatar.zip] Finished downloading! (516549 bytes total)
2024-12-07T10:38:22Z [Bootstrapper::ExtractPackage] Reading content-avatar.zip...
2024-12-07T10:38:22Z [Bootstrapper::DownloadPackage.content-configs.zip] Downloading...
2024-12-07T10:38:22Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-configs.zip
2024-12-07T10:38:22Z [Bootstrapper::ExtractPackage] Read content-avatar.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\avatar\...
2024-12-07T10:38:22Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-configs.zip
2024-12-07T10:38:22Z [Bootstrapper::ExtractPackage] Finished extracting content-avatar.zip
2024-12-07T10:38:22Z [Bootstrapper::DownloadPackage.content-configs.zip] Finished downloading! (261963 bytes total)
2024-12-07T10:38:22Z [Bootstrapper::ExtractPackage] Reading content-configs.zip...
2024-12-07T10:38:22Z [Bootstrapper::DownloadPackage.content-fonts.zip] Downloading...
2024-12-07T10:38:22Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-fonts.zip
2024-12-07T10:38:23Z [Bootstrapper::ExtractPackage] Read content-configs.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\configs\...
2024-12-07T10:38:23Z [Bootstrapper::ExtractPackage] Finished extracting content-configs.zip
2024-12-07T10:38:23Z [Bootstrapper::ExtractPackage] Finished extracting RobloxApp.zip
2024-12-07T10:38:23Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-fonts.zip
2024-12-07T10:38:26Z [Bootstrapper::DownloadPackage.content-fonts.zip] Finished downloading! (6005683 bytes total)
2024-12-07T10:38:26Z [Bootstrapper::ExtractPackage] Reading content-fonts.zip...
2024-12-07T10:38:26Z [Bootstrapper::DownloadPackage.content-models.zip] Downloading...
2024-12-07T10:38:26Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-models.zip
2024-12-07T10:38:26Z [Bootstrapper::ExtractPackage] Read content-fonts.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\fonts\...
2024-12-07T10:38:26Z [Bootstrapper::ExtractPackage] Finished extracting content-fonts.zip
2024-12-07T10:38:26Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-models.zip
2024-12-07T10:38:26Z [Bootstrapper::DownloadPackage.content-models.zip] Finished downloading! (491904 bytes total)
2024-12-07T10:38:26Z [Bootstrapper::ExtractPackage] Reading content-models.zip...
2024-12-07T10:38:26Z [Bootstrapper::DownloadPackage.content-sky.zip] Downloading...
2024-12-07T10:38:26Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-sky.zip
2024-12-07T10:38:26Z [Bootstrapper::ExtractPackage] Read content-models.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\models\...
2024-12-07T10:38:27Z [Bootstrapper::ExtractPackage] Finished extracting content-models.zip
2024-12-07T10:38:27Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-sky.zip
2024-12-07T10:38:28Z [Bootstrapper::DownloadPackage.content-sky.zip] Finished downloading! (2448711 bytes total)
2024-12-07T10:38:28Z [Bootstrapper::ExtractPackage] Reading content-sky.zip...
2024-12-07T10:38:28Z [Bootstrapper::DownloadPackage.content-sounds.zip] Downloading...
2024-12-07T10:38:28Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-sounds.zip
2024-12-07T10:38:28Z [Bootstrapper::ExtractPackage] Read content-sky.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\sky\...
2024-12-07T10:38:28Z [Bootstrapper::ExtractPackage] Finished extracting content-sky.zip
2024-12-07T10:38:29Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-sounds.zip
2024-12-07T10:38:29Z [Bootstrapper::DownloadPackage.content-sounds.zip] Finished downloading! (597244 bytes total)
2024-12-07T10:38:29Z [Bootstrapper::ExtractPackage] Reading content-sounds.zip...
2024-12-07T10:38:29Z [Bootstrapper::DownloadPackage.shaders.zip] Downloading...
2024-12-07T10:38:29Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-shaders.zip
2024-12-07T10:38:29Z [Bootstrapper::ExtractPackage] Read content-sounds.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\sounds\...
2024-12-07T10:38:29Z [Bootstrapper::ExtractPackage] Finished extracting content-sounds.zip
2024-12-07T10:38:30Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-shaders.zip
2024-12-07T10:38:34Z [Bootstrapper::DownloadPackage.shaders.zip] Finished downloading! (8449913 bytes total)
2024-12-07T10:38:34Z [Bootstrapper::ExtractPackage] Reading shaders.zip...
2024-12-07T10:38:34Z [Bootstrapper::DownloadPackage.ssl.zip] Downloading...
2024-12-07T10:38:34Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-ssl.zip
2024-12-07T10:38:34Z [Bootstrapper::ExtractPackage] Read shaders.zip.  Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\content\textures\...
2024-12-07T10:38:42Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-textures3.zip
2024-12-07T10:38:44Z [Bootstrapper::DownloadPackage.content-textures3.zip] Finished downloading! (3713218 bytes total)
2024-12-07T10:38:44Z [Bootstrapper::ExtractPackage] Reading content-textures3.zip...
2024-12-07T10:38:44Z [Bootstrapper::DownloadPackage.content-terrain.zip] Downloading...
2024-12-07T10:38:44Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-terrain.zip
2024-12-07T10:38:44Z [Bootstrapper::ExtractPackage] Read content-textures3.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\PlatformContent\pc\textures\...
2024-12-07T10:38:45Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-terrain.zip
2024-12-07T10:38:45Z [Bootstrapper::DownloadPackage.content-terrain.zip] Finished downloading! (3267 bytes total)
2024-12-07T10:38:45Z [Bootstrapper::ExtractPackage] Reading content-terrain.zip...
2024-12-07T10:38:45Z [Bootstrapper::DownloadPackage.content-platform-fonts.zip] Downloading...
2024-12-07T10:38:45Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-platform-fonts.zip
2024-12-07T10:38:45Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-platform-fonts.zip
2024-12-07T10:38:45Z [Bootstrapper::ExtractPackage] Finished extracting content-textures3.zip
2024-12-07T10:38:45Z [Bootstrapper::ExtractPackage] Read content-terrain.zip.  Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\PlatformContent\pc\fonts\...
2024-12-07T10:38:52Z [Bootstrapper::ExtractPackage] Finished extracting content-platform-fonts.zip
2024-12-07T10:38:52Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-content-platform-dictionaries.zip
2024-12-07T10:38:52Z [Bootstrapper::DownloadPackage.content-platform-dictionaries.zip] Finished downloading! (152263 bytes total)
2024-12-07T10:38:52Z [Bootstrapper::DownloadPackage.extracontent-places.zip] Downloading...
2024-12-07T10:38:52Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-places.zip
2024-12-07T10:38:52Z [AsyncHelpers::ExceptionHandler] An exception occurred while running the task 'extracting content-platform-dictionaries.zip'
2024-12-07T10:38:52Z [App::FinalizeExceptionHandling] System.AggregateException: One or more errors occurred. (The given key 'content-platform-dictionaries.zip' was not present in the dictionary.)
 ---> System.Collections.Generic.KeyNotFoundException: The given key 'content-platform-dictionaries.zip' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Bloxstrap.Bootstrapper.ExtractPackage(Package package)
   --- End of inner exception stack trace ---
2024-12-07T10:38:52Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-places.zip
2024-12-07T10:38:53Z [Bootstrapper::DownloadPackage.extracontent-places.zip] Finished downloading! (1510546 bytes total)
2024-12-07T10:38:53Z [Bootstrapper::ExtractPackage] Reading extracontent-places.zip...
2024-12-07T10:38:53Z [Bootstrapper::DownloadPackage.extracontent-luapackages.zip] Downloading...
2024-12-07T10:38:53Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-luapackages.zip
2024-12-07T10:38:53Z [Bootstrapper::ExtractPackage] Read extracontent-places.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\ExtraContent\places\...
2024-12-07T10:38:53Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-luapackages.zip
2024-12-07T10:38:53Z [Bootstrapper::ExtractPackage] Finished extracting extracontent-places.zip
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Finished extracting content-textures2.zip
2024-12-07T10:38:56Z [Bootstrapper::DownloadPackage.extracontent-luapackages.zip] Finished downloading! (6622887 bytes total)
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Reading extracontent-luapackages.zip...
2024-12-07T10:38:56Z [Bootstrapper::DownloadPackage.extracontent-translations.zip] Downloading...
2024-12-07T10:38:56Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-translations.zip
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Read extracontent-luapackages.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\ExtraContent\LuaPackages\...
2024-12-07T10:38:56Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-translations.zip
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Finished extracting extracontent-luapackages.zip
2024-12-07T10:38:56Z [Bootstrapper::DownloadPackage.extracontent-translations.zip] Finished downloading! (484131 bytes total)
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Reading extracontent-translations.zip...
2024-12-07T10:38:56Z [Bootstrapper::DownloadPackage.extracontent-models.zip] Downloading...
2024-12-07T10:38:56Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-models.zip
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Read extracontent-translations.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\ExtraContent\translations\...
2024-12-07T10:38:56Z [Bootstrapper::ExtractPackage] Finished extracting extracontent-translations.zip
2024-12-07T10:38:56Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-models.zip
2024-12-07T10:39:02Z [Bootstrapper::DownloadPackage.extracontent-models.zip] Finished downloading! (10469290 bytes total)
2024-12-07T10:39:02Z [Bootstrapper::ExtractPackage] Reading extracontent-models.zip...
2024-12-07T10:39:02Z [Bootstrapper::DownloadPackage.extracontent-textures.zip] Downloading...
2024-12-07T10:39:02Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-textures.zip
2024-12-07T10:39:02Z [Bootstrapper::ExtractPackage] Read extracontent-models.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\ExtraContent\models\...
2024-12-07T10:39:02Z [Bootstrapper::ExtractPackage] Finished extracting extracontent-models.zip
2024-12-07T10:39:02Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-extracontent-textures.zip
2024-12-07T10:39:11Z [Bootstrapper::DownloadPackage.extracontent-textures.zip] Finished downloading! (17130170 bytes total)
2024-12-07T10:39:11Z [Bootstrapper::ExtractPackage] Reading extracontent-textures.zip...
2024-12-07T10:39:11Z [Bootstrapper::DownloadPackage.WebView2.zip] Downloading...
2024-12-07T10:39:11Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-WebView2.zip
2024-12-07T10:39:11Z [Bootstrapper::ExtractPackage] Read extracontent-textures.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1\ExtraContent\textures\...
2024-12-07T10:39:11Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-WebView2.zip
2024-12-07T10:39:11Z [Bootstrapper::DownloadPackage.WebView2.zip] Finished downloading! (81240 bytes total)
2024-12-07T10:39:11Z [Bootstrapper::ExtractPackage] Reading WebView2.zip...
2024-12-07T10:39:11Z [Bootstrapper::DownloadPackage.WebView2RuntimeInstaller.zip] Downloading...
2024-12-07T10:39:11Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-WebView2RuntimeInstaller.zip
2024-12-07T10:39:12Z [Bootstrapper::ExtractPackage] Read WebView2.zip. Extracting to %UserProfile%\AppData\Local\Bloxstrap\Versions\version-a2fb906f52d742c1...
2024-12-07T10:39:12Z [Bootstrapper::ExtractPackage] Finished extracting WebView2.zip
2024-12-07T10:39:12Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-WebView2RuntimeInstaller.zip
2024-12-07T10:39:13Z [Bootstrapper::DownloadPackage.WebView2RuntimeInstaller.zip] Finished downloading! (1486447 bytes total)
2024-12-07T10:39:13Z [Bootstrapper::DownloadPackage.redist.zip] Downloading...
2024-12-07T10:39:13Z [HttpClientLoggingHandler::ProcessRequest] GET https://setup.rbxcdn.com/version-a2fb906f52d742c1-redist.zip
2024-12-07T10:39:13Z [HttpClientLoggingHandler::ProcessResponse] 200 OK https://setup.rbxcdn.com/version-a2fb906f52d742c1-redist.zip
2024-12-07T10:39:14Z [Bootstrapper::ExtractPackage] Finished extracting extracontent-textures.zip
2024-12-07T10:39:14Z [Bootstrapper::DownloadPackage.redist.zip] Finished downloading! (1372275 bytes total)
2024-12-07T10:39:14Z [AsyncHelpers::ExceptionHandler] An exception occurred while running the task 'extracting redist.zip'
2024-12-07T10:39:14Z [App::FinalizeExceptionHandling] System.AggregateException: One or more errors occurred. (The given key 'redist.zip' was not present in the dictionary.)
 ---> System.Collections.Generic.KeyNotFoundException: The given key 'redist.zip' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Bloxstrap.Bootstrapper.ExtractPackage(Package package)
   --- End of inner exception stack trace ---
2024-12-07T10:39:15Z [Bootstrapper::SetStatus] Configuring Roblox...
2024-12-07T10:45:10Z [App::Terminate] Terminating with exit code 1603 (ERROR_INSTALL_FAILURE)
2024-12-07T10:45:10Z [JsonManager<Settings>::Save] Save request ignored
2024-12-07T10:45:10Z [JsonManager<State>::Save] Save request ignored
2024-12-07T10:45:10Z [NotifyIconWrapper::Dispose] Disposing NotifyIcon

update
https://bloxstraplabs.com/

i dont want to lose the graphic bar feature

it doesn't even work anymore
and that's too bad

bruh i hate this i lag now i hate my crusty dusty ahh pc

Update Bloxstrap.

no i dont want to

no i dont want to

then it wont work


specify json output for a couple az commands that were missing it

Which issue this PR addresses:

n/a
What this PR does / why we need it:

A couple az commands in the create_miwi_env_file function in hack/devtools/local_dev_env.sh script didn't specify to use json output, so changing the default output type (eg: to table) breaks things
Test plan for issue:

Manually run
Is there any documentation that needs to be updated for this PR?

No, this is a bugfix of a dev script
How do you know this will function as expected in production?

It will not go to production, it's a dev script

Thanks for this fix! I took a quick look through the rest of the file and found one more that could be fixed at the same time: the az deployment group list on line 302.

Thanks, added!

/azp run ci


[Text Analytics] Add support for default language in automatic language detection

Resolves: #32738

Added the AbstractSummaryOptions.AutoDetectionDefaultLanguage property.
Added the AnalyzeActionsOptions.AutoDetectionDefaultLanguage property.
Added the AnalyzeHealthcareEntitiesOptions.AutoDetectionDefaultLanguage property.y.
Added the ExtractSummaryOptions.AutoDetectionDefaultLanguage property.
Added the MultiLabelClassifyOptions.AutoDetectionDefaultLanguage property.
Added the RecognizeCustomEntitiesOptions.AutoDetectionDefaultLanguage property.
Added the SingleLabelClassifyOptions.AutoDetectionDefaultLanguage property.

API change check
APIView has identified API level changes in this PR and created following API reviews.
Azure.AI.TextAnalytics


Most RKI sequences lack submission date

Most RKI sequences lack submission date since move to the new format.
I've opened an issue in the RKI repo to let them know that most submission dates have gone missing with the new format: https://github.com/robert-koch-institut/SARS-CoV-2-Sequenzdaten_aus_Deutschland/issues/50
If this doesn't get fixed upstream we can patch the submission dates back using dates from the old format, but let's wait for a bit to save us unnecessary work.
Originally posted by @corneliusroemer in https://github.com/nextstrain/ncov-ingest/issues/409#issuecomment-1630894026

RKI has said they have fixed the issue. We should check on Wednesday 26th of July, as the changes will take some time to propagate.


BREAKING CHANGE - xDisk, xDiskAccessPath, xWaitForDisk support for Disk Unique Id - Fixes #81

Fixes:

https://github.com/PowerShell/xStorage/issues/80
https://github.com/PowerShell/xStorage/issues/81
Partially: https://github.com/PowerShell/xStorage/issues/86

This PR changes xDisk, xDiskAccessPath and xWaitForDisk:

Removes DiskNumber parameter and replaces with DiskId and DiskIdType. The reason why this was done is because DiskNumber is not a reliable method of identifying disks (it can change between reboots as has been shown). This change will allow support for both DiskNumber and Disk Unique Id to identify disks.

Details:

Fixed style violations in xDisk.
BREAKING CHANGE: Renamed xDisk parameter DiskNumber to DiskId to enable it to
contain either DiskNumber or UniqueId - See Issue 81.
Added DiskIdType parameter to xDisk to enable specifying the type of identifer
the DiskId parameter contains - See Issue 81.
Changed xDisk to use xDiskAccessPath pattern to fix issue with Windows Server
2016 - See Issue 80.
BREAKING CHANGE: Renamed xDiskAccessPath parameter DiskNumber to DiskId to
enable it to contain either DiskNumber or UniqueId - See Issue 81.
Added DiskIdType parameter to xDiskAccessPath to enable specifying the type
of identifer the DiskId parameter contains - See Issue 81.
Fixed incorrect logging messages in xDiskAccessPath when changing volume label.
BREAKING CHANGE: Renamed xWaitForDisk parameter DiskNumber to DiskId to
enable it to contain either DiskNumber or UniqueId - See Issue 81.
Added DiskIdType parameter to xWaitForDisk to enable specifying the type
of identifer the DiskId parameter contains - See Issue 81.

This change is

Codecov Report

Merging #87 into dev will increase coverage by <1%.
The diff coverage is 83%.

@@        Coverage Diff         @@
##           dev   #87    +/-   ##
==================================
+ Coverage   90%   90%   +<1%
==================================
  Files        5     5
  Lines      600   625    +25
==================================
+ Hits       540   563    +23
- Misses      60    62     +2

Hmm. Looks like I'll need to improve the code coverage a bit :grin:
I'll let you know when I'm done.

Ok. Done. Coverage increased. Slightly :grin:

Breakfast pause in review. :)

Reviewed 5 of 31 files at r1.
Review status: 5 of 30 files reviewed at latest revision, 3 unresolved discussions.

CHANGELOG.md, line 17 at r2 (raw file):

- Fixed unit tests for MSFT_xDiskAccessPath resource to be compatible with
  Pester 4.0.3.
- Fixed style violations in xDisk.

It's hard to see where changes to one resource start and end.
Maybe continue doing as under <IP_ADDRESS>?
- xDisk
  - BREAKING CHANGE: Change 1
  - Change 2
  ...
- xDiskAccessPath
  - BREAKING CHANGE: Change 1
  - Change 2
  ...

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 125 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 462 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Comments from Reviewable

Comments so far.
It's hard ignoring style issue in unchanged code, so might be a few extra changes :grin:

Reviewed 3 of 31 files at r1.
Review status: 8 of 30 files reviewed at latest revision, 60 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 27 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 47 at r2 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 55 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 68 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 68 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 81 at r2 (raw file):

    $DriveLetter = Assert-DriveLetterValid -DriveLetter $DriveLetter

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 146 at r2 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 154 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 167 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 167 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 180 at r2 (raw file):

    $DriveLetter = Assert-DriveLetterValid -DriveLetter $DriveLetter

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 215 at r2 (raw file):

    switch ($disk.PartitionStyle)
    {
        "RAW"

single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 228 at r2 (raw file):

            break
        } # "RAW"
        "GPT"

single qoutes. And new line before this one.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 238 at r2 (raw file):

            break
        } # "GPT"
        default

new line before this one.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 252 at r2 (raw file):

    # Check if the disk has an existing partition assigned to the drive letter
    $assignedPartition = $partition |
            Where-Object -Property DriveLetter -EQ DriveLetter

Change -EQ  to -eq (lower letter)

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 312 at r2 (raw file):

                    ) -join '' )

                $partitionParams["Size"] = $Size

single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 323 at r2 (raw file):

                    ) -join '' )

                $partitionParams["UseMaximumSize"] = $true

single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 329 at r2 (raw file):

            $partition = $disk | New-Partition @partitionParams

            # After creating the partition it can take a few seconds for it to become writeable

Change this to a comment-block
<#
    Comment
#>

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 333 at r2 (raw file):

            $start = Get-Date
            $timeout = (Get-Date) + (New-Timespan -Second 30)
            While ($partition.IsReadOnly -and (Get-Date) -lt $timeout)

Why so complex with Get-Date, why not just loop for a max of 30 times?

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 333 at r2 (raw file):

            $start = Get-Date
            $timeout = (Get-Date) + (New-Timespan -Second 30)
            While ($partition.IsReadOnly -and (Get-Date) -lt $timeout)

Lower case w for while?

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 482 at r2 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 490 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 503 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 503 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 516 at r2 (raw file):

    $DriveLetter = Assert-DriveLetterValid -DriveLetter $DriveLetter

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.schema.mof, line 10 at r2 (raw file):

    [Write, Description("Specifies the size of new volume.")] Uint64 Size;
    [Write, Description("Define volume label if required.")] String FSLabel;
    [Write, Description("Specifies the allocation unit size to use when formatting the volume.")] uint32 AllocationUnitSize;

Upper U on Uint32

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 27 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 47 at r2 (raw file):

    param
    (
        [parameter(Mandatory)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 47 at r2 (raw file):

    param
    (
        [parameter(Mandatory)]

Change to Mandatory = $true

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 55 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 65 at r2 (raw file):

        $FSLabel,

        [System.UInt32]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 68 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 129 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 150 at r2 (raw file):

    param
    (
        [parameter(Mandatory)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 150 at r2 (raw file):

    param
    (
        [parameter(Mandatory)]

Change to Mandatory = $true

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 158 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 168 at r2 (raw file):

        $FSLabel,

        [System.UInt32]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 171 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 458 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 478 at r2 (raw file):

    param
    (
        [parameter(Mandatory)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 478 at r2 (raw file):

    param
    (
        [parameter(Mandatory)]

Change to Mandatory = $true

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 486 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 496 at r2 (raw file):

        $FSLabel,

        [System.UInt32]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 499 at r2 (raw file):

        $AllocationUnitSize,

        [ValidateSet("NTFS","ReFS")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.schema.mof, line 10 at r2 (raw file):

    [Write, Description("Specifies the size of new volume.")] Uint64 Size;
    [Write, Description("Define volume label if required.")] String FSLabel;
    [Write, Description("Specifies the allocation unit size to use when formatting the volume.")] uint32 AllocationUnitSize;

Upper U on Uint32

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 24 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 38 at r2 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 42 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 49 at r2 (raw file):

        $RetryIntervalSec = 10,

        [System.UInt32]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 75 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 88 at r2 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 92 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single qoutes

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 99 at r2 (raw file):

        $RetryIntervalSec = 10,

        [System.UInt32]

Add [Parameter()] to all non-mandatory parameters.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 153 at r2 (raw file):

    .PARAMETER DiskIdType
    Specifies the identifier type the DiskId contains.

Maybe add the default here as well?

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 167 at r2 (raw file):

    param
    (
        [parameter(Mandatory = $true)]

Upper P in Parameter, do this throughout

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 171 at r2 (raw file):

        $DiskId,

        [ValidateSet("Number","UniqueId")]

Single quotes

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 178 at r2 (raw file):

        $RetryIntervalSec = 10,

        [System.UInt32]

Add [Parameter()] to all non-mandatory parameters.

Comments from Reviewable

Thanks @johlju - I really appreciate you taking the time to review this! :grin: I'll start working my way through your changes and hopefully get it all done before the end of the weekend.

@johlju - I've made the changes. I've tried to use global search/replace wherever possible, so hopefully I've fixed some issues in the files you haven't yet reviewed.

Review status: 4 of 36 files reviewed at latest revision, 60 unresolved discussions.

CHANGELOG.md, line 17 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
It's hard to see where changes to one resource start and end.
Maybe continue doing as under <IP_ADDRESS>?
- xDisk
  - BREAKING CHANGE: Change 1
  - Change 2
  ...
- xDiskAccessPath
  - BREAKING CHANGE: Change 1
  - Change 2
  ...

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 27 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 47 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

I did a global search/replace so this was changed 27 times throughout all files.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 55 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 68 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 68 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done. I also corrected the Parameter blocks of all other resources as well.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 81 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 125 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 146 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 154 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 167 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 167 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 180 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 215 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 228 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
single qoutes. And new line before this one.

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 238 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
new line before this one.

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 252 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change -EQ  to -eq (lower letter)

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 312 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 323 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 329 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change this to a comment-block
<#
    Comment
#>

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 333 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Why so complex with Get-Date, why not just loop for a max of 30 times?

Just to be accurate. Because Get-Partition can take a up to a second (or sometimes longer) to query, looping 30 times might result in a >30 second timeout. Not the end of the world, but a 30 second timeout should be 30 seconds :grin:

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 333 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Lower case w for while?

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 462 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 482 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 490 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 503 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 503 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 516 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.schema.mof, line 10 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper U on Uint32

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 27 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 47 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 47 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to Mandatory = $true

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 55 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 65 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 68 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 129 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 150 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 150 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to Mandatory = $true

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 158 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 168 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 171 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 458 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 478 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 478 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Change to Mandatory = $true

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 486 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 496 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 499 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.schema.mof, line 10 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper U on Uint32

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 24 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 38 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 42 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 49 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 75 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 88 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 92 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single qoutes

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 99 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 153 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Maybe add the default here as well?

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 167 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper P in Parameter, do this throughout

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 171 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 178 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add [Parameter()] to all non-mandatory parameters.

Done.

Comments from Reviewable

Reviewed 18 of 31 files at r1.
Review status: 19 of 36 files reviewed at latest revision, 76 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 333 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Why so complex with Get-Date, why not just loop for a max of 30 times?

Never mind, ignore the previous comment. :smile:
Figured out why when I took a break. If Get-Partition takes a while to return there will be longer wait than 30 seconds.

Modules/xStorage/DSCResources/MSFT_xDisk/en-us/MSFT_xDisk.strings.psd1, line 5 at r2 (raw file):

    SettingDiskMessage = Setting disk with {0} '{1}' status for drive letter '{2}'.
    SetDiskOnlineMessage = Setting disk with {0} '{1}' online.
    SetDiskReadwriteMessage = Setting disk with {0} '{1}' to read/write.

Upper W on Write? (Have not checked the code if the same problem is there)

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 81 at r2 (raw file):

    $AccessPath = Assert-AccessPathValid -AccessPath $AccessPath -Slash

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 184 at r2 (raw file):

    $AccessPath = Assert-AccessPathValid -AccessPath $AccessPath -Slash

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 517 at r2 (raw file):

        ) -join '' )

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/en-us/MSFT_xDiskAccessPath.strings.psd1, line 5 at r2 (raw file):

    SettingDiskMessage = Setting disk with {0} '{1}' status for access path '{2}'.
    SetDiskOnlineMessage = Setting disk with {0} '{1}' online.
    SetDiskReadwriteMessage = Setting disk with {0} '{1}' to read/write.

Upper W on Write. Not checkoed code so problem might be there as well.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 110 at r2 (raw file):

    $diskFound = $false

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 187 at r2 (raw file):

        ) -join '' )

    $diskIdParameter = @{ $DiskIdType = $DiskId }

Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 3 at r2 (raw file):

for disk with

for Disk 2 with

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 6 at r2 (raw file):

...volume on Disk 3...

....volume on Disk 3 with unique ID xxx-xxx-xx ...

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 23 at r2 (raw file):

        }

        xDisk GVolume

Add Depends for this one as well.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 40 at r2 (raw file):

        }

        xDisk DataVolume

SHould'n Disk3 have a xWaitForDisk Disk3?

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 42 at r2 (raw file):

        xDisk DataVolume
        {
             DiskId = '5E1E50A401000000001517FFFF0AEB84'

Should "Disk 3" have the same UniqueID?
Maybe add a comment after the UniqueID # Disk 3  to visualize it more? Or Change 'DataVolume' to 'Disk3'

Modules/xStorage/Examples/Resources/xDIskAccessPath/1-xDiskAccessPath_InitializeDataDiskWithAccessPath.ps1, line 1 at r2 (raw file):

<#

This folder has a typo in it 'xDIskAccessPath' (Upper 'I').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)

tests/unit/MSFT_xDisk.tests.ps1, line 1 at r2 (raw file):

$script:DSCModuleName      = 'xStorage'

This folder has a typo in it 'Tests' (missing Upper 'T').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)
And for some reason the path is in lower-case as well, but not for the other unit tests. See 'tests/unit'. Maybe we just ignore that for now. :)

Tests/Unit/MSFT_xDiskAccessPath.tests.ps1, line 1 at r2 (raw file):

$script:DSCModuleName      = 'xStorage'

This folder has a typo in it 'Tests' (missing Upper 'T').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)

Tests/Unit/MSFT_xWaitForDisk.tests.ps1, line 1 at r2 (raw file):

$script:DSCModuleName      = 'xStorage'

This folder has a typo in it 'Tests' (missing Upper 'T').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)

Comments from Reviewable

Gone thru all files! Waiting for you to fix stuff now. :smile:

Reviewed 3 of 31 files at r1, 2 of 2 files at r2, 12 of 12 files at r3.
Review status: all files reviewed at latest revision, 42 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 120 at r3 (raw file):

        [Parameter()]
        [ValidateSet("ISO","VHD","VHDx","VHDSet")]

Well since you got more resource involved in this PR, then I let you work some more...  Why is the config file dot sourced here when we compile it in the tests below?

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 56 at r3 (raw file):

            #region DEFAULT TESTS
            It 'should compile without throwing' {

'Should compile and apply the MOF without throwing' (this is throughout)

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 81 at r3 (raw file):

            It 'should be able to call Get-DscConfiguration without throwing' {
                { Get-DscConfiguration -Verbose -ErrorAction Stop } | should Not throw

Shouldn't Should be with upper 'S'? I would have written it as Should Not Throw, but just my preference. So I don't know if this is something we should enforce.
Anyway... I think either this should be should not throw, Should Not Throw.
TestGuideline is buggy to, some have Should Be and some have Should be (lower 'b'). We should fix that TestGuideline too :smile:
https://github.com/PowerShell/DscResources/blob/master/TestsGuidelines.md

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 106 at r3 (raw file):

                                DiskIdType  = 'Number'
                                FSLabel     = $FSLabelB
                                Size        = 200MB

Should Size be set here? Expected Size not to be set here so that the if ($Node.Size) would be used in the configuration file.

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 135 at r3 (raw file):

            It 'should have 3 partitions on disk' {
                ($disk | Get-Partition).Count | Should Be 3

We only created 2 partitions (?), so the third is created by system. My thought is that maybe comment saying what the third partition came from?

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 227 at r3 (raw file):

                                DiskIdType    = 'UniqueId'
                                FSLabel       = $FSLabelB
                                Size          = 200MB

Same comment about size as above

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 256 at r3 (raw file):

            It 'should have 3 partitions on disk' {
                ($disk | Get-Partition).Count | Should Be 3

Some comment about 3 partitions as above

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 63 at r3 (raw file):

            #region DEFAULT TESTS
            It 'Should compile without throwing' {

Same comment as previous test. Thoughout

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 166 at r3 (raw file):

                                DiskIdType  = 'Number'
                                FSLabel     = $FSLabelB
                                Size        = 200MB

Same comment about size as previous test

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 194 at r3 (raw file):

            It 'should have 3 partitions on disk' {
                ($disk | Get-Partition).Count | Should Be 3

Same comment about 3 partitions as previous test

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 302 at r3 (raw file):

                                DiskId      = $disk.UniqueId
                                DiskIdType  = 'UniqueId'
                                FSLabel     = $FSLabelA

This does not have Size. Is that correct?

Tests/Integration/MSFT_xWaitForDisk.Integration.Tests.ps1, line 29 at r3 (raw file):

    . $ConfigFile -Verbose -ErrorAction Stop

    Describe "$($script:DSCResourceName)_Integration" {

Shouldn't it also be a test when it can't find a disk and 'throws with the correct error' after 1 retry?

Tests/TestHelpers/CommonTestHelper.psm1, line 131 at r3 (raw file):

    {
        Write-Verbose -Message "Integration tests cannot be run because Hyper-V Components not installed." -Verbose
        Return $false

lower case 'r' on return

Tests/TestHelpers/CommonTestHelper.psm1, line 133 at r3 (raw file):

        Return $false
    }
    Return $true

lower case 'r' on return

Comments from Reviewable

Review status: 28 of 36 files reviewed at latest revision, 42 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xDisk/en-us/MSFT_xDisk.strings.psd1, line 5 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper W on Write? (Have not checked the code if the same problem is there)

Did a global search/replace - the code had the same case problem - but fixed now.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 81 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 184 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 517 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 120 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Well since you got more resource involved in this PR, then I let you work some more... Haha :wink:
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 125 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 130 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 259 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 264 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 269 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 403 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 408 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 413 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 500 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xMountImage/MSFT_xMountImage.psm1, line 505 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Single quotes here

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 110 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/DSCResources/MSFT_xWaitForDisk/MSFT_xWaitForDisk.psm1, line 187 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Hash table properties on separate row
https://github.com/PowerShell/DscResources/blob/master/StyleGuidelines.md#correct-format-for-hashtables-or-objects

Done.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 3 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

for disk with

for Disk 2 with

Done.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 40 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
SHould'n Disk3 have a xWaitForDisk Disk3?

Done.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 42 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should "Disk 3" have the same UniqueID?
Maybe add a comment after the UniqueID # Disk 3  to visualize it more? Or Change 'DataVolume' to 'Disk3'

You're right - it should have had a different ID. I've corrected that and added some comments to indicate the different disks.

Modules/xStorage/Examples/Resources/xDIskAccessPath/1-xDiskAccessPath_InitializeDataDiskWithAccessPath.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This folder has a typo in it 'xDIskAccessPath' (Upper 'I').
 It seems to have worked.

Comments from Reviewable

Thanks for the review @johlju.

Review status: 28 of 36 files reviewed at latest revision, 42 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/en-us/MSFT_xDiskAccessPath.strings.psd1, line 5 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Upper W on Write. Not checkoed code so problem might be there as well.

Done.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 6 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

...volume on Disk 3...

....volume on Disk 3 with unique ID xxx-xxx-xx ...

Done.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 23 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Add Depends for this one as well.

I've examined these examples in more detail and reworked them a bit.

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 32 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Curious since I haven't written any of these yet. Why is the config file dot sourced here when we compile it in the tests below?

This is just the standard integration test pattern. But moving it into the compile test block would mean it would be dotsource more than once (because many integration tests compile multiple times with different Config Data values). Does that make sense?

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 56 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
'Should compile and apply the MOF without throwing' (this is throughout)

Done.

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 81 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Shouldn't Should be with upper 'S'? I would have written it as Should Not Throw, but just my preference. So I don't know if this is something we should enforce.
Anyway... I think either this should be should not throw, Should Not Throw.
TestGuideline is buggy to, some have Should Be and some have Should be (lower 'b'). We should fix that TestGuideline too :smile:
https://github.com/PowerShell/DscResources/blob/master/TestsGuidelines.md

I've done a global search/replace on this and fixed them all (there were a lot across the whole resource module).

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 106 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should Size be set here? Expected Size not to be set here so that the if ($Node.Size) would be used in the configuration file.

Unfortunately there is a bug in this version that causes this to not work. I've logged this (https://github.com/PowerShell/xStorage/issues/86). I should be able to fix this, but it will prevent volume mount points from being changed (e.g. change d: to e: or c:\Data1 -> c:\Data2) because when size is not specified the only way to identify a drive is by mount point/drive letter. So leave this with me for a little bit to try and fix this.
The newer xDiskEx, xPartition and xVolume will not have this sort of problem (another reason to deprecate xDisk).

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 135 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We only created 2 partitions (?), so the third is created by system. My thought is that maybe comment saying what the third partition came from?

That is correct. I've added a comment to clarify.

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 227 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Same comment about size as above

See above.

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 256 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Some comment about 3 partitions as above

Done.

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 63 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Same comment as previous test. Thoughout

Done.

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 166 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Same comment about size as previous test

Done.

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 194 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Same comment about 3 partitions as previous test

Done.

Tests/Integration/MSFT_xDiskAccessPath.Integration.Tests.ps1, line 302 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This does not have Size. Is that correct?

That is correct because we're technically remounting an existing disk so the goal is to test that the disk is still remountable (I had some difficulty getting this to work in early versions of xDiskAccessPath - which is why I created this specific regression integration test).

Tests/Integration/MSFT_xWaitForDisk.Integration.Tests.ps1, line 29 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Shouldn't it also be a test when it can't find a disk and 'throws with the correct error' after 1 retry?

I'm not sure that is a test we should be performing in integration testing because we would need to start testing for all sorts of other error conditions that might be returned by the *-Disk, *-Partition and *-Volume cmdlets- which would get very time consuming (and heavy on resource usage). Exception path tests might be better performed in unit tests - and far less costly. In general I think we should only be performing happy path tests in our integration suite - but now we're getting into test theory :grin:

Tests/TestHelpers/CommonTestHelper.psm1, line 131 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
lower case 'r' on return

Done.

Tests/TestHelpers/CommonTestHelper.psm1, line 133 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
lower case 'r' on return

Done.

tests/unit/MSFT_xDisk.tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This folder has a typo in it 'Tests' (missing Upper 'T').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)
And for some reason the path is in lower-case as well, but not for the other unit tests. See 'tests/unit'. Maybe we just ignore that for now. :)

I've tried the rename (had to do a double rename). I'm not sure it has worked though...

Tests/Unit/MSFT_xDiskAccessPath.tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This folder has a typo in it 'Tests' (missing Upper 'T').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)

This folder looks OK to me (the other one was bad).

Tests/Unit/MSFT_xWaitForDisk.tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This folder has a typo in it 'Tests' (missing Upper 'T').
Look at 'git mv' to see it that helps you https://git-scm.com/docs/git-mv (never used, so let me know how it worked)

Same again - this folder is OK.

Comments from Reviewable

@johlju - just testing the fix for #86 which I'll include in this PR. Nearly there! :grin:

@johlju - Ok, I've included the fix for #86 - which also addresses your comments regarding the missing size from the integration tests.

Reviewed 14 of 22 files at r4, 8 of 8 files at r5.
Review status: 38 of 39 files reviewed at latest revision, 10 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 298 at r5 (raw file):

            } # if

            if ($partition)

This entire if-block feels wrong at this location. If the user did not specify Size it will print verbose message referencing "size". This if-block should be in the if ($Size)-block, after the correct partition is searched for (when the user actually   assigned Size a value).

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 299 at r5 (raw file):

            } # if

            if ($partition)

This entire if-block feels wrong at this location. If the user did not specify Size it will print verbose message referencing "size". This if-block should be in the if ($Size)-block, after the correct partition is searched for (when the user actually   assigned Size a value).

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 6 at r2 (raw file):

Disk 3

disk 3 (lower case 'd' since that is how 'disk 2' is written)

Modules/xStorage/Examples/Resources/xDiskAccessPath/1-xDiskAccessPath_InitializeDataDiskWithAccessPath.ps1, line 1 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
I attempted to use the git mv and I had to do a two step process (rename to a temp folder name and then rename it to the final name). It seems to have worked.

Awesome! Good to know it worked when doing it in two steps!

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 32 at r3 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
This is just the standard integration test pattern. But moving it into the compile test block would mean it would be dotsource more than once (because many integration tests compile multiple times with different Config Data values). Does that make sense?

Ah, okay, I thought this one loaded it from file system each time. But when I looked closer I see now that it doesn't.
& "$($script:DSCResourceName)_Config" `
                         -OutputPath $TestDrive `
                         -ConfigurationData $configData

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 135 at r3 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
That is correct. I've added a comment to clarify.

Can't see the comment.

Tests/Integration/MSFT_xWaitForDisk.Integration.Tests.ps1, line 29 at r3 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
I'm not sure that is a test we should be performing in integration testing because we would need to start testing for all sorts of other error conditions that might be returned by the *-Disk, *-Partition and *-Volume cmdlets- which would get very time consuming (and heavy on resource usage). Exception path tests might be better performed in unit tests - and far less costly. In general I think we should only be performing happy path tests in our integration suite - but now we're getting into test theory :grin:

OK. My thought was more to test each logic path in the code.  But I agree that can be tested in the unit tests. Lets keep them happy tests only :grinning:

tests/unit/MSFT_xDisk.tests.ps1, line 1 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
I've tried the rename (had to do a double rename). I'm not sure it has worked though...

Nah, didn't work. :/ Try again or leave it? Up to you. :smile:

Tests/Unit/MSFT_xDiskAccessPath.tests.ps1, line 1 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
This folder looks OK to me (the other one was bad).

The other one had folders wrong, this and two other has '.tests.ps1'  worng, should be '.Tests.ps1' (upper 'T')
But I we don't have to hold this PR because of that.

Tests/Unit/MSFT_xWaitForDisk.tests.ps1, line 1 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
Same again - this folder is OK.

Se previous comment

Comments from Reviewable

@johlju - Ok hopefully good to go :grin:

Review status: 29 of 39 files reviewed at latest revision, 4 unresolved discussions.

Modules/xStorage/DSCResources/MSFT_xDisk/MSFT_xDisk.psm1, line 298 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This entire if-block feels wrong at this location. If the user did not specify Size it will print verbose message referencing "size". This if-block should be in the if ($Size)-block, after the correct partition is searched for (when the user actually   assigned Size a value).

I've refined this a little bit, but the code still needs to be done in two blocks - find partition and create partition if not found. So the separate checks for Size need to be kept separate (and we need one in each block because the bahviour changes). I did make some slight improvements, but I've kept the logs in there because this can be very helpful for debugging.

Modules/xStorage/DSCResources/MSFT_xDiskAccessPath/MSFT_xDiskAccessPath.psm1, line 299 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This entire if-block feels wrong at this location. If the user did not specify Size it will print verbose message referencing "size". This if-block should be in the if ($Size)-block, after the correct partition is searched for (when the user actually   assigned Size a value).

I've refined this a little bit, but the code still needs to be done in two blocks - find partition and create partition if not found. So the separate checks for Size need to be kept separate (and we need one in each block because the bahviour changes). I did make some slight improvements, but I've kept the logs in there because this can be very helpful for debugging.

Modules/xStorage/Examples/Resources/xDisk/2-xDisk_InitializeDataDiskUsingUniqueId.ps1, line 6 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

Disk 3

disk 3 (lower case 'd' since that is how 'disk 2' is written)

Done.

tests/unit/MSFT_xDisk.tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Nah, didn't work. :/ Try again or leave it? Up to you. :smile:

What I might have to do is try two PR's to fix this (rename everything, then rename it back). So if you're up for it I'll do this after this PR is merged.

Tests/Unit/MSFT_xDiskAccessPath.tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
The other one had folders wrong, this and two other has '.tests.ps1'  worng, should be '.Tests.ps1' (upper 'T')
But I we don't have to hold this PR because of that.

I've fixed the file name case, but correcting the folder case seems to not work :(

Tests/Unit/MSFT_xWaitForDisk.tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Se previous comment

Same as previous! :)

Tests/Integration/MSFT_xDisk.Integration.Tests.ps1, line 135 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can't see the comment.

Doh. Sorry. Done.

Comments from Reviewable

Reviewed 16 of 16 files at r6.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

tests/unit/MSFT_xDisk.tests.ps1, line 1 at r2 (raw file):
Previously, PlagueHO (Daniel Scott-Raynsford) wrote…
What I might have to do is try two PR's to fix this (rename everything, then rename it back). So if you're up for it I'll do this after this PR is merged.

OK. Should be enought with a delete and a push, and then add and a push. But let's do that in another PR. Saw that more files changed from '.Tests.' to '.test.' on this last push. Should they not be upper 'T'?
Either, let's do this in another PR, so we can get this merged.

Comments from Reviewable

Review status: 27 of 39 files reviewed at latest revision, all discussions resolved.

Tests/Unit/MSFT_xDisk.Tests.ps1, line 1 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
OK. Should be enought with a delete and a push, and then add and a push. But let's do that in another PR. Saw that more files changed from '.Tests.' to '.test.' on this last push. Should they not be upper 'T'?
Either, let's do this in another PR, so we can get this merged.

@johlju - I decided to try again with a different method of renaming the files... it worked, but it now thinks they're new files :cry:
So you'll need to sign them off. They aren't changed in any other way. But the filename case has been corrected now.

Comments from Reviewable

Awesome! I will check them off! 😄

Reviewed 24 of 24 files at r7.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable


Fix typo

Fixes #36865

@Potapy4 : Thanks for your contribution! The author(s) have been notified to review your proposed change.


Netstandard support

The logic and complexity around figuring out the project information has been moved in to a new Project class. This keeps it all self-contained and lets the engine work with csproj/vbproj/xproj files and lets it use the information in the project files to figure out the path to the binary. I also added a new method to ProjectEngine called GetProjectPackages which takes a Project instance instead of a path to the project.lock/packages.config file so the engine has the responsibility of figuring out which method to call (GetProjectJsonPackages or GetPackagesConfigPackages).
All of this also let me simplify the command line args by dropping the fourth argument. Arg 3 is still a semi-colon delimited list, but it can be a list of just project files  "tests\SampleProjJsonPcl\SampleProjJsonPcl.csproj;" or project file and configuration '"tests\SampleProjJsonPcl\SampleProjJsonPcl.csproj=Debug;"

Cool, thx, will take a look in a little bit!

One question on this so far -- because it tries to calculate the binary path from the project file, how would that work if the OutputPath was changed externally - like calling msbuild /p:OutputPath="c:\foo"?

Hmmm...good point. I don't think my change would handle that. Maybe I misunderstood what those two command line args were for then. I thought they worked like this:

args[3] was the path the csproj file and args[4] was the path to the compiled binary that was being included.

Given that understanding, it made sense to me to pull that path from the project file (not taking in to account that it could be changed via msbuild command line). I should be able to change this so it takes the binary file path as the second part of the pair (instead of pulling it from the project file) instead without much effort and I may even be able to make it "smart" enough to handle making the binary path optional (in which case it would pull from the project file.)

Yep, your understanding is correct, it is to get the compiled binary(s) being included. I just couldn't come up with a reliable way of getting them statically from the project files. That's why I had them as a parameter (that was for the most part automatic).
If you could please update the PR to pull in the binary files that way (and/or some smarts), that'd be great!

I updated the pull request so it can work with just the path to the project file, the project file and path to the binary, or the project file and a configuration name. I also updated the command line section in the readme.

Looks good -- I made a few code cleanups based on R# and pushed that back to the netstandard-pr branch. One final thing -- if the parameters are going to be projectfile=assemblyPath for the last grouping, can you please add an MSBuild mechanism to join the current @(NuSpecProjectFile) @(NuSpecLibContent) into those pairs? That's the last missing piece to support the current set off functionality.

The reason, BTW, for those input arrays is also to support being overridden in the csproj file if the user has a more advanced scenario that needs to package multiple dlls into a single nuget package.

I'll take a look at adding that in the morning.

I'm also ok if it's easier just to keep them as two separate parameters and zip them in code the right way.
I was also thinking of using the new System.Commandline stuff to do the argument parsing. I can add that to the backlog :)


Add docker proxy settings

If proxy is defined on the server, docker pull will fail.

Thanks for the PR, but I'm not going to merge this.  I think the use case is a bit too limited.  I'd personally prefer that users explicitly modify their own proxy settings -- this is trying to be too magical.

All enterprises use proxy, so case is not a bit too limited.

Some but not all; there are lots of way to funnel traffic.
The main point I want to make here is that your proxy settings should be explicitly set, not implicitly parsed from the Ansible environment.  This happens to work in your environment, but it's totally possible this is way too niche for others.
This is a role -- meaning it can be part of a larger configuration.  I think post_tasks is a fine place to configure proxy settings for Docker.

we need this fix too  and I'm happy to improve this, so it's configurable.
I see >2 solutions
(1) small addendum to the current PR:
Just add a new default variable: docker_http_proxy_from_ansible_env: false
and add an extra 'and' clause in the task's when
(2) More flexible solution, requiring multiple new variables
(that could still by default be set to @majkinetor 's nice choice of re'using ansible_env.http_proxy )
Add 3 new default vars:
docker_http_proxy_enabled: false
docker_http_proxy: "{{ansible_env.http_proxy | default('') }}"
docker_https_proxy: "{{ansible_env.https_proxy | default('') }}"

with updated 'when':
when:
 - docker_http_proxy_enabled
 - docker_http_proxy !=''

@angstwad chance to get the proxy feature merged then?  I assume better with solution (2)
ps: I'm open for different impl.  just should be flexible :)

I brought this feature back 'alive' , required some fixes -> https://github.com/angstwad/docker.ubuntu/pull/122
and already used it successfully to deploy proxy settings (and it works 👍 )
Awaiting feedback for the final choice  on howto configure it


change the default tab key binding?

I bind the tab to other oporation,how can i change the default tab behavior?

If you put your custom keybinding into your User keymap (packages/User/Default ({OS}).keymap) file, that will override the plugin setting.