CameraSystem cleanup

As @TheBerryBeast noted, the RenderSystem adds a CameraSystem if one isn't present yet.
However, users could add one right after adding the RenderSystem - this will not work (nor will it ever). We should find a way to handle this. Perhaps letting the RenderSystem crash if no CameraSystem was added before adding the RenderSystem? That would require everyone to explicitely add the CameraSystem, but will also prevent people from accidentally adding it afterwards, thinking it'll work.
Any thoughts, workarounds or suggestions are welcome - from anyone.

I think we could take some inspiration from negroni and have an "add default systems" sort of function. common.DefaultSystems(w *World) or something?
The function would add all of the "recommended systems" in the correct order. This might be worth moving to another issue as it doesn't completely fix this one.

I don't see any problem with the RenderSystem, or any System, automatically adding other systems they require so long as those systems have no entities. The real issue here is attempting to add multple CameraSystems, since a game can only have one camera. I think we should check at CameraSystem.New() if there's more than one, and then panic if there is. We should also update the documentation for the RenderSystem to say that it adds a CameraSystem if one is not present as well as documentation for the CameraSystem to say that it panics if more than one CameraSystem is added to the World. Maybe have the panic mention that RenderSystem automatically adds a CameraSystem so people will see that and go "oh."


Support timezones in version 1.0.0

A had a problem running sidekiq-cron on my development machine because I was located in EST time zone, while the sidekiq-cron stored my time as UTC but parsed it as EST, causing a "Last enqueued" date in the future, like this:

This meant that the cron job that should have been running once every minute was actually running once every 4 hours.
Before the change:
[17] pry(main)> Sidekiq::Cron::Job.all
=> [#<Sidekiq::Cron::Job:0x00007feb6e3f1d60
  @active_job=false,
  @active_job_queue_name_delimiter="",
  @active_job_queue_name_prefix="",
  @args=[],
  @cron="* * * * *",
  @description="",
  @fetch_missing_args=false,
  @klass="GithubTokenRefresherJob",
  @last_enqueue_time=2018-07-18 14:38:32 -0400,
  @message="{\"retry\":true,\"queue\":\"default\",\"class\":\"GithubTokenRefresherJob\",\"args\":[]}",
  @name="github_token_refresher_job",
  @queue="default",
  @queue_name_with_prefix="default",
  @status="enabled">]

Notice that last_enqueue_time contained wrong timezone info, it was 10:38AM at EST timezone, which is 14:38 in UTC, but it stores the timezone as EST, which is the problem.
After the change:
[6] pry(main)> Sidekiq::Cron::Job.all
=> [#<Sidekiq::Cron::Job:0x00007f8a30e76410
  @active_job=false,
  @active_job_queue_name_delimiter="",
  @active_job_queue_name_prefix="",
  @args=[],
  @cron="* * * * *",
  @description="",
  @fetch_missing_args=false,
  @klass="GithubTokenRefresherJob",
  @last_enqueue_time=2018-07-18 14:40:22 +0000,
  @message="{\"retry\":true,\"queue\":\"default\",\"class\":\"GithubTokenRefresherJob\",\"args\":[]}",
  @name="github_token_refresher_job",
  @queue="default",
  @queue_name_with_prefix="default",
  @status="enabled">]

Notice that last_enqueue_time now has the correct timezone. I ran this in my local environment, and it seems to work as intended.

gh-209 and gh-210

Coverage remained the same at 91.711% when pulling a17e6bc8420d33cfe566c7b79b6a71c322bddec8 on peterzhu2118:master into b215aa2abfabc1ca31372770c72738667f1703cf on ondrejbartas:master.


Failed to save permissions for 'demo/product.txt' on Azure Storage explorer

Storage Explorer Version:
Build Number:
Platform/OS:
Architecture:
Regression From:
Bug Description

Steps to Reproduce

Expected Experience

Actual Experience

Additional Context

please help on this issue

To better help you, can you please elaborate what the issues you were having?  What did you do, on which version of Storage Explorer, on what platform?  What permissions do you have in terms of accessing Azure storage?

Closing due too lack of response.


KARNATAKA state missing from Deep Dive Line graph

The line graph in deep dive section was having Karnataka state a few days ago and now its missing. Might be a bug or an overlap with other states.

Deep-dive tab needs serious upgrades. Making a user-friendly way to compare all the states in teh state daily graph would be one key thing Making it issue available for interested folks to pick up and work on!

@sudevschiz  Can't we have interactive dashboard like this one

In States - Total Cases, showing all states(lines) at one place is looking clumsy, it is difficult to differentiate. Why can't we have a slider inside the card to select state. For showing all states together(as it is there now) we can give an option 'ALL'

I have added a "Toggle Selection" button in #1003 fixing #836. That may make it more useable, when you want to compare only few states

You guys showing only the top 10 states so that's why Karnataka is not visible because it's in 11th place. I have two suggestions for this issue.
1)I can show the top 12 states so that Karnataka will be visible
2) or else, we can show the states which have the count of corona cases >150.
NOTE : Showing all the states might be clumsy.
Suggest me the right one

@ninjakx I suggest you create an issue with the Enhancement template with proper specifications to keep the discussions relevant.  And if the suggestion is accepted then we can assign you the task and let you submit the PR for the same.

I am saying about the above  issue (#854)

i can solve this issue, please assign this to me

@shuklaayush Made a PR

Closing in favor of #812


custom icon color

default callout icons are colored - is this also possible in custom icons?

icons by default are colored by the color of the background and title. They can be customized further with CSS. Look for everything here that has the word "icon" in it.


Bot will try to play ip grabbing links

The bot will try to play any link including ip grabbing ones

What's the issue?  The bot can play MP3 or other music files given the direct link to the bot as a feature.  I'm not sure what you mean by "ip grabbing ones".

nothing we can do about


backup seed phrase screen doesn't fit in iPhone 5s

@James-Sangalli would you mention which class or screenshot so we know which one it is?


Libraries!

Some helpful npm libraries for our servers:

request provides all request data in a flat object
reply provides standard error messages
db provides some database querying tools
client generates a client library from an openapi spec

bump to see if the bot will re-run


Easier Contributions.

Hi.
I work for Gitpod(an online Open Source VS Code like IDE which is free for Open Source) and we're currently on a mission to simplify contributors/maintainers onboarding for cool Open Source projects.
This Pr adds Gitpod config to the repo to fully automate the dev setup for this project i.e with this config anyone interested in contributing to this project would be able to start a workspace where it will automatically:

clone the react-query repo.
install all the dependencies in / and /docs.
run yarn start in the root(/) to Auto-build files.
run yarn dev in /docs.

This way anyone interested in contributing can start straight away without any friction.
This is how it looks:

You can give it a try on my fork of the repo via the following link:
https://github.com/nisarhassan12/react-query
A lot of projects out there are already using Gitpod to simplify contributors/maintainers onboarding experience some of them are:

https://github.com/freeCodeCamp/freeCodeCamp
https://github.com/facebook/docusaurus
https://github.com/mozilla/pdf.js/
https://github.com/gatsbyjs/gatsby/
https://github.com/mobxjs/mobx/

You can find a brief list of them at https://contribute.dev

Please let me know if there is anything that can be improved. I would to help.

:tada: This PR is included in version 3.2.0-beta.1 :tada:
The release is available on:

npm package (@beta dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


structural/naming alterations?

Following up with #2, maybe we should reconsider the structure of spopt. The current structure (of non-base classes, etc) is:

allocate
region
route

I propose we change allocate to facility_location (or facloc for shorthand). I intend to add in covering models (LSCP, MCLP, etc.) and those don't generally fall under "location allocation". Also, I would like to add in a Transportation Problem class, so maybe changing route to transport, which would house transportation-oriented models, is a good idea. So the new structure would be:

facloc
region
transport

we should try to avoid abbreviations or portmaneaus in our APIs, I think.
If allocate is too specific, what about just locate?

we should try to avoid abbreviations or portmanteaus in our APIs, I think.
If allocate is too specific, what about just locate?

I think your suggestion sounds nice.

locate
region
transport

Do you agree with having transport house the Transportation Problem, the Traveling Salesperson Problem, etc.? I was trying to think of a good descriptive to:

have a clear distinction  from locate; and
encompass both transportation-style and routing-style problems

Or maybe just stick with route?

settled on:

locate
region
transport

Will push up new structure soon

Addressing in #38


How to change the port from 443 to 8000? I am trying to run the setup on my Linux server.

"Linux - Port :443 blocked by default, may have to change the port to 8000 to get working"
How to do this? Sorry. I am a newbie

index.js const PORT = 443; line 11 change for 8000 and be happy @satcit-me, if you update, this change is gone

just pushed a change to make this easier. run PORT=8000 npm run start to run if off that port!
make sure to pull my latest changes~ also try running sh ./test-installation.sh to test ur config

@keldenl nice update

@satcit-me lmk if u got it working, if so, i'll go-ahead and close it

Closing this as the PORT was added as a supported env var.


Filesize

Following the problems we have currently with the upload (PROG and SAVE), I thought it would be usefull to show the size of each file stored on the SD card.
Using a second array of string for the labels that are actually displayed, it is still possible to flash the file from the SD card to the internal flash.

Agreed! Any quality-of-life stuff in the firmware makes sense for now.


Update peerDependencies to support Grunt 1.0

Update peerDependencies to support Grunt 1.0
Hello,
This is an automated issue request to update the peerDependencies for your Grunt plugin.
We ask you to merge this and publish a new release on npm to get your plugin working in Grunt 1.0!
Read more here: http://gruntjs.com/blog/2016-02-11-grunt-1.0.0-rc1-released#peer-dependencies
Also on Twitter: https://twitter.com/gruntjs/status/700819604155707392
If you have any questions or you no longer have time to maintain this plugin, then please let us know in this thread.
Thanks for creating and working on this plugin!
(P.S. Close this PR if it is a mistake, sorry)

Please merge this. Now that 1.0.0 has been released, this is breaking builds.

I also need this patch, otherwise will fork this and patch it myself.

Anyone even active here? :troll:

+1

+1

+1

+1

+1

+1

It seems like this package might be ready for a fork. Perhaps the @gruntjs org would be willing to take this on.

+1

Hey guys, just use https://github.com/sindresorhus/grunt-shell which is up-to-date :)

Fixed in latest release.  Thank you for the PR!


Add Victor Hugo Kontent boilerplate

Add Victor Hugo Kontent boilerplate into starter kits.

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

We have moved the Hugo site source to its own repository, so we would appreciate if you could open this PR directly in hugoDocs:  https://github.com/gohugoio/hugoDocs/pulls


For node js

Can I use this library in node.js?

maybe that's what you're looking for ?
import { DeviceUUID } from 'device-uuid';
var uuid = new DeviceUUID().get();
console.log("uuid : " + uuid);


[stable/elasticsearch] support elasticsearch 7 & graceful termination on master nodes

What this PR does / why we need it:

elasticsearch.yml config for es 7
elasticsearch-graceful-termination-handler for master nodes https://github.com/elastic/helm-charts/pull/119

Which issue this PR fixes
(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

fixes #8785

Special notes for your reviewer:
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

[x] DCO signed
[x] Chart Version bumped
[x] Variables are documented in the README.md

/assign @desaintmartin

I didn't bump es version because es7 doesn't support rolling upgrade.

You'll also need to sign your commits!

signed all commits

@desaintmartin Refactored configmap.yaml

@desaintmartin  rewrote master termination handling process

/test pull-charts-e2e


Add log_duration_is_not_set query Closes #1647 and typos fix

Closes #1647
Check if the 'log_duration' is set to 'on'

Scan submitted to Checkmarx

Checkmarx SAST - Scan Summary & Details
Cx-SAST Summary
Total of 4 vulnerabilities 0 High 3 Medium 1 Low 0 Info
Violation Summary
 3 Medium
View more details on Checkmarx UI
Cx-SAST Details

Lines
Severity
Category
File
Link

30
Medium
Path_Traversal
cmd/builder/main.go
Checkmarx

83
Medium
Denial_Of_Service_Resource_Exhaustion
test/main_test.go
Checkmarx

52
Medium
Denial_Of_Service_Resource_Exhaustion
pkg/kics/service.go
Checkmarx


KAFKA-2948: Remove unused topics from producer metadata set

If no messages are sent to a topic during the last refresh interval or if UNKNOWN_TOPIC_OR_PARTITION error is received, remove the topic from the metadata list. Topics are added to the list on the next attempt to send a message to the topic.

@rajinisivaram I think we also need a test on the consumer side that ensures we always set subscribed or assigned topics to never expire.

@ewencp Thank you for the review. I have made the updates and also added a test for the consumer side.

@rajinisivaram Thanks for the update. I left a couple nitpick comments, but overall this seems a lot clearer.

@hachikuji Thank you for the reviews. Have made the changes.

@hachikuji @ewencp What is the status of this one, are you happy with it?

Started a system tests build:
https://jenkins.confluent.io/job/kafka_system_tests_branch_builder/

@ijuma This was reviewed a long time ago and I think all the review comments have been addressed. I have rebased and started another system test run (https://jenkins.confluent.io/job/system-test-kafka-branch-builder/448/). Do you mind taking a look at the changes? Thank you.

There were two system test failures in connect in the test run above and both look like unrelated intermittent test failures. @hachikuji @ewencp @ijuma Can you review this PR? Thank you.

Sorry for the delay @rajinisivaram, I left a few comments that should be easy to address. @hachikuji, would you like to take a look as well?

@ijuma Thank you for the review. I have made all except one of the changes you suggested. Have left a comment for the remaining one.

Thanks @rajinisivaram, LGTM.

@rajinisivaram The eager removal of unknown topics seems questionable to me. It looks like we just end up complicating other parts of the code to add the removed topics back into the Metadata instance. I'm wondering if this actually solves the problem of unknown topic log spam. In particular, it seems like we would still log the unknown topic on each metadata fetch in KafkaProducer.waitOnMetadata.

The latest update seems fine to me, @hachikuji can you please check?

LGTM

LGTM, merging to trunk.

Thanks for your patience @rajinisivaram.


cmd/ctrl+l should select all the text in the URL field

STR:
Open a tab
Press cmd/ctrl+l
Expected
URL bar is focused and text is selected
Actual
URL bar is focused and cursor is at end (no selection)

This seems to only when navigation has happened outside of the URL bar, so if I open a new tab, then click a link or submit a form, then do cmd/ctrl+l I can reproduce

Fixed in the new frontend.


Resources for managing DHCP option values (server, scope, reserved IP and policy)

Hello, here is the resource for managing DHCP option values. This could substitute the MSFT_xDhcpServerOption resource.
Fixes #11

This change is

@PlagueHO I will review this, but do you have time to take a glance at these four new resources to see if you see anything that jumps out? I think you have more knowledge of DHCP than me.

I will continue the review in a bit.

Review status: 0 of 18 files reviewed, 2 unresolved discussions (waiting on @marciorschneider)

README.md, line 19 at r4 (raw file):

xDhcpServerOptions

Are you seeing these resource deprecating MSFT_xDhcpServerOption? In that case I think we should prefix this description with (DEPRECATED).

README.md, line 50 at r4 (raw file):

xDhcpServerOption

Are you seeing these resource deprecating MSFT_xDhcpServerOption? In that case I think we should suffix this section with (DEPRECATED), e.g. xDhcpServerOption (DEPRECATED) .

Comments from Reviewable

Awesome @marciorschneider - and @johlju will do. Just got to finish the big Language one over in ComputerManagementDsc

@PlagueHO Great! I know, it take a while to review these big PR's :)

@marciorschneider Awesome work on this! Code looks great! Review comments are for the most part just minor style changes.

Reviewed 7 of 22 files at r2, 1 of 5 files at r3, 10 of 10 files at r4.
Review status: all files reviewed, 89 unresolved discussions (waiting on @marciorschneider)

a discussion (no related file):
I don't see the need to add the x prefix on these new resources, I think we should remove that so we are closer to renaming this resource module. I suggest we call it DhcpPolicyOptionValue, MSFT_DhcpPolicyOptionValue etc. What do you think?
From Naming.md in DSCResource:

The "x" prefix is no longer required. Resources that include the prefix are free to deprecate the convention going forward.
https://github.com/PowerShell/DscResources/blob/master/Naming.md#deprecated-experimental

DSCResources/OptionValueHelper.psm1, line 2 at r4 (raw file):

# Localized messages
data LocalizedData

Could we move these strings to a .strings.psd1 under an en-US folder?
See https://github.com/PowerShell/DscResources/blob/master/BestPractices.md#localization
When doing this, it would also be more appropriate to move the module to a new folder location; /Modules/DhcpServerDsc.OptionValueHelper

DSCResources/OptionValueHelper.psm1, line 25 at r4 (raw file):

$policyWithScopeRemoveValueMessage

I think this should not have a $ in front of the key.

DSCResources/OptionValueHelper.psm1, line 42 at r4 (raw file):

}

Extra blank row here.

DSCResources/OptionValueHelper.psm1, line 43 at r4 (raw file):

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/OptionValueHelper.psm1, line 53 at r4 (raw file):

        The option ID.

    .PARAMETER OptionValue

This is not among the functions parameters. It can be removed from here.

DSCResources/OptionValueHelper.psm1, line 127 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

This parameter is not needed here? It's not used in the code (and can then also be removed from the Get-TargetResource in the resources).

DSCResources/OptionValueHelper.psm1, line 137 at r4 (raw file):

    #endregion Input Validation

  # Checking if option needs to be configured for server, DHCP scope, Policy or reservedIP

We should indent this comment.

DSCResources/OptionValueHelper.psm1, line 146 at r4 (raw file):

             $serverGettingValueMessage = $localizedData.ServerGettingValueMessage -f $OptionId, $VendorClass, $UserClass
             Write-Verbose $serverGettingValueMessage
             $currentConfiguration = Get-DhcpServerv4OptionValue -OptionId $OptionId -VendorClass $VendorClass -UserClass $UserClass -ErrorAction SilentlyContinue

Can we use splatting here (and throughout) to shorten the rows, some are very long?

DSCResources/OptionValueHelper.psm1, line 163 at r4 (raw file):

            Write-Verbose $policyGettingValueMessage

            # Policy can exist on server or scope level, so we need to address both cases

We should use comment block for these comments that is more than one row.
<#
    Text ...
#>

DSCResources/OptionValueHelper.psm1, line 164 at r4 (raw file):

If ScopeId is $null

Do we need this comment? there are two code paths and comment is only mentioning one.

DSCResources/OptionValueHelper.psm1, line 219 at r4 (raw file):

}

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/OptionValueHelper.psm1, line 306 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

Mandatory parameter should not have default values.

DSCResources/OptionValueHelper.psm1, line 315 at r4 (raw file):

        {
            # Trying to get the option value
            $currentConfiguration = Get-TargetResourceHelper -ApplyTo 'Server' -OptionId $OptionId -VendorClass $VendorClass -UserClass $UserClass -AddressFamily $AddressFamily -Ensure $Ensure

Can we use splatting here (and throughout) to shorten the rows, some are very long?

DSCResources/OptionValueHelper.psm1, line 335 at r4 (raw file):

                }
            }

Remove extra blank line

DSCResources/OptionValueHelper.psm1, line 490 at r4 (raw file):

}

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/OptionValueHelper.psm1, line 578 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

Mandatory parameters should not have default values.

DSCResources/OptionValueHelper.psm1, line 586 at r4 (raw file):

        'Server'
        {
            $currentConfiguration = Get-TargetResourceHelper -ApplyTo 'Server' -OptionId $OptionId -VendorClass $VendorClass -UserClass $UserClass -AddressFamily $AddressFamily -Ensure $Ensure

Can we use splatting here (and throughout) to shorten the rows, some are very long?

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 9 at r4 (raw file):

Import-Module -Name $modulePathOptionValueHelper

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 19 at r4 (raw file):

        The ID of the option.

    .PARAMETER Value

This is not among the functions parameters. It can be removed from here.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 41 at r4 (raw file):

[parameter(

Parameter (upper 'P'). Throughout.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 51 at r4 (raw file):

[parameter(Mandatory = $false)]

We should write these as [Parameter()]. Throughout.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 69 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 72 at r4 (raw file):

    )

    $result = Get-TargetResourceHelper -ApplyTo 'Policy' -PolicyName $PolicyName -OptionId $OptionId -ScopeId $ScopeId -VendorClass $VendorClass -UserClass '' -AddressFamily $AddressFamily -Ensure $Ensure

Can we use splatting here (and throughout) to shorten the row?

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 77 at r4 (raw file):

 }

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 145 at r4 (raw file):

}

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.schema.mof, line 4 at r4 (raw file):

uint32

UInt32 (upper 'UI').

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 9 at r4 (raw file):

Import-Module -Name $modulePathOptionValueHelper

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 19 at r4 (raw file):

        The ID of the option.

    .PARAMETER Value

This is not among the functions parameters. It can be removed from here.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 35 at r4 (raw file):

        When set to 'Absent', the option will be removed.
#>

Remove extra line

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 42 at r4 (raw file):

[parameter(

Parameter (upper 'P'). Throughout.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 70 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 73 at r4 (raw file):

    )

    $result = Get-TargetResourceHelper -ApplyTo 'ReservedIP' -ReservedIP $ReservedIP -OptionId $OptionId -VendorClass $VendorClass -UserClass $UserClass -AddressFamily $AddressFamily -Ensure $Ensure

Can we use splatting here (and throughout) to shorten the row?

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 78 at r4 (raw file):

 }

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 104 at r4 (raw file):

        When set to 'Absent', the option will be removed.
#>

Remove extra line

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 148 at r4 (raw file):

}

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.schema.mof, line 5 at r4 (raw file):

uint32

UInt32 (upper 'UI').

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 9 at r4 (raw file):

Import-Module -Name $modulePathOptionValueHelper

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 19 at r4 (raw file):

        The ID of the option.

    .PARAMETER Value

This is not among the functions parameters. It can be removed from here.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 35 at r4 (raw file):

        When set to 'Absent', the option will be removed.
#>

Remove extra blank line.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 42 at r4 (raw file):

[parameter(

Parameter (upper 'P'). Throughout.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 70 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 73 at r4 (raw file):

    )

    $result = Get-TargetResourceHelper -ApplyTo 'Scope' -ScopeId $ScopeId -OptionId $OptionId -VendorClass $VendorClass -UserClass $UserClass -AddressFamily $AddressFamily -Ensure $Ensure

Can we use splatting here (and throughout) to shorten the row?

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 78 at r4 (raw file):

 }

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 104 at r4 (raw file):

        When set to 'Absent', the option will be removed.
#>

Remove extra line

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 148 at r4 (raw file):

}

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.schema.mof, line 5 at r4 (raw file):

uint32

UInt32 (upper 'UI').

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 9 at r4 (raw file):

Import-Module -Name $modulePathOptionValueHelper -Force

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 16 at r4 (raw file):

        The ID of the option.

    .PARAMETER Value

This is not among the functions parameters. It can be removed from here.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 32 at r4 (raw file):

        When set to 'Absent', the option will be removed.
#>

Remove extra line

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 39 at r4 (raw file):

[parameter(

Parameter (upper 'P'). Throughout.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 62 at r4 (raw file):

        [ValidateSet('Present','Absent')]
        [String]
        $Ensure = 'Present'

This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 65 at r4 (raw file):

    )

    $result = Get-TargetResourceHelper -ApplyTo 'Server' -OptionId $OptionId -VendorClass $VendorClass -UserClass $UserClass -AddressFamily $AddressFamily -Ensure $Ensure

Can we use splatting here (and throughout) to shorten the row?

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 70 at r4 (raw file):

 }

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 93 at r4 (raw file):

        When set to 'Absent', the option will be removed.
#>

Remove extra line

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 132 at r4 (raw file):

}

   <#

We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.schema.mof, line 4 at r4 (raw file):

uint32

UInt32 (upper 'UI').

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 1 at r4 (raw file):

configuration Sample_xDhcpsServerScope_NewScope

Could we add comment-based to the new examples, at least .SYNOPSIS (add a .DESCRIPTION if the description gets to long).
Throughout all the new examples.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 1 at r4 (raw file):

configuration Sample_xDhcpsServerScope_NewScope

We should name it Example, that way test framework can validate the examples (once op-in). E.g configuration Example.
Throughout all the new examples.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 3 at r4 (raw file):

xDHCpServer

xDhcpServer (lower-case 'hcp').
Throughout all the new examples.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 3 at r4 (raw file):

configuration Sample_xDhcpsServerScope_NewScope
{
    Import-DscResource -module xDHCpServer

Should also import PSDscResources (for WindowsFeature).
Throughout all the new examples.

Samples/xDhcpReservedIPOptionValue/xDhcpReservedIPOptionValue.ps1, line 3 at r4 (raw file):

-ModuleVersion

We can remove this.

Samples/xDhcpScopeOptionValue/xDhcpScopeOptionValue.ps1, line 10 at r4 (raw file):

#set scope gateway

We should have space between # and text. Throughout.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 19 at r4 (raw file):

#endregion HEADER

Remove extra line.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 86 at r4 (raw file):

-ParameterFilter

One extra space before this parameter.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 87 at r4 (raw file):

-ModuleName OptionValueHelper

Non-blocking: Curious, do you need to to use -ModuleName (throughout), it doesn't work without? (it should I think, but can be wrong).

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 98 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also tests so that Get-TargetResource returns the correct values.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 98 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also have a test for when Get-TargetResource returns 'Absent'

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 99 at r4 (raw file):

                $result = Get-TargetResource @testParams -Ensure 'Present'
                $result -is [System.Collections.Hashtable] | Should Be $true;

We can use BeOfType here as well (as thew previous test does).

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 112 at r4 (raw file):

 $result  |

An extra space here

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 19 at r4 (raw file):

#endregion HEADER

Remove extra line

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 83 at r4 (raw file):

-ParameterFilter

One extra space before this parameter.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 95 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also tests so that Get-TargetResource returns the correct values.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 95 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also have a test for when Get-TargetResource returns 'Absent'

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 96 at r4 (raw file):

                $result = Get-TargetResource @testParams -Ensure 'Present'
                $result -is [System.Collections.Hashtable] | Should Be $true;

We can use BeOfType here as well (as thew previous test does).

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 109 at r4 (raw file):

                $result = Test-TargetResource @testParams -Ensure 'Present' -Value $value
                $result  | Should BeOfType [System.Boolean]

An extra space here

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 19 at r4 (raw file):

#endregion HEADER

Remove extra line

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 86 at r4 (raw file):

-ParameterFilter

One extra space before this parameter.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 98 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also tests so that Get-TargetResource returns the correct values.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 98 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also have a test for when Get-TargetResource returns 'Absent'

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 99 at r4 (raw file):

                $result = Get-TargetResource @testParams -Ensure 'Present'
                $result -is [System.Collections.Hashtable] | Should Be $true;

We can use BeOfType here as well (as thew previous test does).

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 112 at r4 (raw file):

                $result = Test-TargetResource @testParams -Ensure 'Present' -Value $value
                $result  | Should BeOfType [System.Boolean]

An extra space here

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 19 at r4 (raw file):

#endregion HEADER

Remove extra line

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 81 at r4 (raw file):

-ParameterFilter

One extra space before this parameter.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 93 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also tests so that Get-TargetResource returns the correct values.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 93 at r4 (raw file):

            It 'Returns a "System.Collections.Hashtable" object type' {

                $result = Get-TargetResource @testParams -Ensure 'Present'

We should also have a test for when Get-TargetResource returns 'Absent'

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 94 at r4 (raw file):

                $result = Get-TargetResource @testParams -Ensure 'Present'
                $result -is [System.Collections.Hashtable] | Should Be $true;

We can use BeOfType here as well (as thew previous test does).

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 107 at r4 (raw file):

                $result = Test-TargetResource @testParams -Ensure 'Present' -Value $value
                $result  | Should BeOfType [System.Boolean]

An extra space here

Comments from Reviewable

Codecov Report

:exclamation: No coverage uploaded for pull request base (dev@f06becc). Click here to learn what that means.
The diff coverage is 100%.

@@          Coverage Diff           @@
##             dev      #43   +/-   ##
======================================
  Coverage       ?   66.33%
======================================
  Files          ?       11
  Lines          ?      603
  Branches       ?        4
======================================
  Hits           ?      400
  Misses         ?      199
  Partials       ?        4

Impacted Files
Coverage Δ

...pPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1
100% <100%> (ø)

...pServerOptionValue/MSFT_DhcpServerOptionValue.psm1
100% <100%> (ø)

...dIPOptionValue/MSFT_DhcpReservedIPOptionValue.psm1
100% <100%> (ø)

...hcpScopeOptionValue/MSFT_DhcpScopeOptionValue.psm1
100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f06becc...c9a30b4. Read the comment docs.

Done with the fixes :)

Review status: 0 of 20 files reviewed, 89 unresolved discussions (waiting on @johlju and @marciorschneider)

a discussion (no related file):
Previously, johlju (Johan Ljunggren) wrote…
I don't see the need to add the x prefix on these new resources, I think we should remove that so we are closer to renaming this resource module. I suggest we call it DhcpPolicyOptionValue, MSFT_DhcpPolicyOptionValue etc. What do you think?
From Naming.md in DSCResource:

The "x" prefix is no longer required. Resources that include the prefix are free to deprecate the convention going forward.
https://github.com/PowerShell/DscResources/blob/master/Naming.md#deprecated-experimental

I agree, it's a step forward.

README.md, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

xDhcpServerOptions

Are you seeing these resource deprecating MSFT_xDhcpServerOption? In that case I think we should prefix this description with (DEPRECATED).

Yes, I'll put an example on how use the MSFT_xDhcpScopeOptionValue to substitute it.

README.md, line 50 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

xDhcpServerOption

Are you seeing these resource deprecating MSFT_xDhcpServerOption? In that case I think we should suffix this section with (DEPRECATED), e.g. xDhcpServerOption (DEPRECATED) .

Done.

DSCResources/OptionValueHelper.psm1, line 2 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Could we move these strings to a .strings.psd1 under an en-US folder?
See https://github.com/PowerShell/DscResources/blob/master/BestPractices.md#localization
When doing this, it would also be more appropriate to move the module to a new folder location; /Modules/DhcpServerDsc.OptionValueHelper

Done, for importing the localization data I've used the "CommonResourceHelper.psm1" helper, like the "SqlServerDsc" module.

DSCResources/OptionValueHelper.psm1, line 25 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

$policyWithScopeRemoveValueMessage

I think this should not have a $ in front of the key.

Done.

DSCResources/OptionValueHelper.psm1, line 42 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Extra blank row here.

Done.

DSCResources/OptionValueHelper.psm1, line 43 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

done.

DSCResources/OptionValueHelper.psm1, line 53 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This is not among the functions parameters. It can be removed from here.

Done.

DSCResources/OptionValueHelper.psm1, line 127 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This parameter is not needed here? It's not used in the code (and can then also be removed from the Get-TargetResource in the resources).

Done.

DSCResources/OptionValueHelper.psm1, line 137 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should indent this comment.

Done.

DSCResources/OptionValueHelper.psm1, line 146 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the rows, some are very long?

Done.

DSCResources/OptionValueHelper.psm1, line 163 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should use comment block for these comments that is more than one row.
<#
    Text ...
#>

done.

DSCResources/OptionValueHelper.psm1, line 164 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

If ScopeId is $null

Do we need this comment? there are two code paths and comment is only mentioning one.

done.

DSCResources/OptionValueHelper.psm1, line 219 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/OptionValueHelper.psm1, line 306 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Mandatory parameter should not have default values.

Done.

DSCResources/OptionValueHelper.psm1, line 315 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the rows, some are very long?

Done.

DSCResources/OptionValueHelper.psm1, line 335 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra blank line

Done.

DSCResources/OptionValueHelper.psm1, line 490 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/OptionValueHelper.psm1, line 578 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Mandatory parameters should not have default values.

Done.

DSCResources/OptionValueHelper.psm1, line 586 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the rows, some are very long?

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 9 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This is not among the functions parameters. It can be removed from here.

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 41 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

[parameter(

Parameter (upper 'P'). Throughout.

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 51 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

[parameter(Mandatory = $false)]

We should write these as [Parameter()]. Throughout.

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 69 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 72 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the row?

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 77 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 145 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.schema.mof, line 4 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

uint32

UInt32 (upper 'UI').

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 9 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This is not among the functions parameters. It can be removed from here.

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 35 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 42 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

[parameter(

Parameter (upper 'P'). Throughout.

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 70 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 73 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the row?

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 78 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 104 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.psm1, line 148 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpReservedIPOptionValue/MSFT_xDhcpReservedIPOptionValue.schema.mof, line 5 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

uint32

UInt32 (upper 'UI').

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 9 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This is not among the functions parameters. It can be removed from here.

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 35 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra blank line.

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 42 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

[parameter(

Parameter (upper 'P'). Throughout.

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 70 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 73 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the row?

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 78 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 104 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.psm1, line 148 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpScopeOptionValue/MSFT_xDhcpScopeOptionValue.schema.mof, line 5 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

uint32

UInt32 (upper 'UI').

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 9 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 16 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This is not among the functions parameters. It can be removed from here.

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 32 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 39 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

[parameter(

Parameter (upper 'P'). Throughout.

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 62 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This parameter is not needed here? We should not add unused non-mandatory parameters. Also remove from comment-based-help

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 65 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Can we use splatting here (and throughout) to shorten the row?

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 70 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 93 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.psm1, line 132 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should move this back one indent. And .SYNOPSISon the next row seems out of place by one space too.

Done.

DSCResources/MSFT_xDhcpServerOptionValue/MSFT_xDhcpServerOptionValue.schema.mof, line 4 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

uint32

UInt32 (upper 'UI').

Done.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 1 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Could we add comment-based to the new examples, at least .SYNOPSIS (add a .DESCRIPTION if the description gets to long).
Throughout all the new examples.

Done.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 1 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

configuration Sample_xDhcpsServerScope_NewScope

We should name it Example, that way test framework can validate the examples (once op-in). E.g configuration Example.
Throughout all the new examples.

Done.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 3 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

xDHCpServer

xDhcpServer (lower-case 'hcp').
Throughout all the new examples.

Done.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 3 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should also import PSDscResources (for WindowsFeature).
Throughout all the new examples.

Done.

Samples/xDhcpReservedIPOptionValue/xDhcpReservedIPOptionValue.ps1, line 3 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-ModuleVersion

We can remove this.

Done.

Samples/xDhcpScopeOptionValue/xDhcpScopeOptionValue.ps1, line 10 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

#set scope gateway

We should have space between # and text. Throughout.

Done.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line.

Done.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 86 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-ParameterFilter

One extra space before this parameter.

Done.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 87 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-ModuleName OptionValueHelper

Non-blocking: Curious, do you need to to use -ModuleName (throughout), it doesn't work without? (it should I think, but can be wrong).

Yes, I don't why but without the -Modulename the mocking doesn't work with the helper module.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 98 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also tests so that Get-TargetResource returns the correct values.

Done.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 98 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also have a test for when Get-TargetResource returns 'Absent'

Done.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 99 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We can use BeOfType here as well (as thew previous test does).

Done.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 112 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

 $result  |

An extra space here

Done.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 83 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-ParameterFilter

One extra space before this parameter.

Done.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 95 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also tests so that Get-TargetResource returns the correct values.

Done.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 95 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also have a test for when Get-TargetResource returns 'Absent'

Done.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 96 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We can use BeOfType here as well (as thew previous test does).

Done.

Tests/Unit/MSFT_xDhcpReservedIPOptionValue.Tests.ps1, line 109 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
An extra space here

Done.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 86 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-ParameterFilter

One extra space before this parameter.

Done.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 98 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also tests so that Get-TargetResource returns the correct values.

Done.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 98 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also have a test for when Get-TargetResource returns 'Absent'

Done.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 99 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We can use BeOfType here as well (as thew previous test does).

Done.

Tests/Unit/MSFT_xDhcpScopeOptionValue.Tests.ps1, line 112 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
An extra space here

Done.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 19 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Remove extra line

Done.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 81 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-ParameterFilter

One extra space before this parameter.

Done.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 93 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also tests so that Get-TargetResource returns the correct values.

Done.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 93 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should also have a test for when Get-TargetResource returns 'Absent'

Done.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 94 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We can use BeOfType here as well (as thew previous test does).

Done.

Tests/Unit/MSFT_xDhcpServerOptionValue.Tests.ps1, line 107 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
An extra space here

Done.

Comments from Reviewable

Awesome work resolving those - just a few new minor ones. :smiley:

Reviewed 20 of 20 files at r5.
Review status: all files reviewed, 9 unresolved discussions (waiting on @johlju and @marciorschneider)

README.md, line 19 at r5 (raw file):

xDhcpServerOption

Could you please submit an issue to track removing this deprecated resource in a future release?

en-US/OptionValueHelper.strings.psd1, line 1 at r5 (raw file):

# Localized resources for helper module OptionValueHelper.

This files should be in the folder /Modules/en-US instead ('en-US' should be in the same folder as the module it belongs to).

Modules/CommonResourceHelper.psm1, line 1 at r5 (raw file):

<#

Can you bring in the unit tests for CommonresourceHelper.psm1 as well? There are unit test in the SqlServerDsc repo too (might need to change the path in the tests where it find the module though).

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 29 at r5 (raw file):

        The option definition address family (IPv4 or IPv6). Currently only the IPv4 is supported.

    .PARAMETER Ensure

We should remove this parameter from the comment-based help as it was removed.

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 65 at r5 (raw file):

$Params

Can we write this out as $parameters (also with lower-case 'p), to avoid abbreviations. Throughout, and throughout the new resources.

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 68 at r5 (raw file):

    $Params.Remove('Ensure')
    $result = Get-TargetResourceHelper -ApplyTo 'Policy' -UserClass '' @Params
    $result

We should only return (all) the properties in the schema.mof. The helper function returns more properties.
Throughout the new resources.

Tests/Unit/MSFT_DhcpPolicyOptionValue.Tests.ps1, line 106 at r5 (raw file):

                $result = Get-TargetResource @testParams
                $result.Ensure | Should -Be 'Absent'

We should test so that the rest of the hash table properties is returned with the correct values when Ensure -eq 'Absent.
Throughout the unit tests for the new resources.

DSCResources/MSFT_xDhcpPolicyOptionValue/MSFT_xDhcpPolicyOptionValue.psm1, line 99 at r4 (raw file):

        The option definition address family (IPv4 or IPv6). Currently only the IPv4 is supported.

    .PARAMETER Ensure

Parameter Ensure should have been kept in the comment-based help for this function.

Samples/xDhcpPolicyOptionValue/xDhcpPolicyOptionValue.ps1, line 3 at r4 (raw file):
Previously, marciorschneider (Marcio Schneider) wrote…
Done.

xDhcpServer (upper'D'). :slightly_smiling_face:
Throughout the new examples.

Tests/Unit/MSFT_xDhcpPolicyOptionValue.Tests.ps1, line 87 at r4 (raw file):
Previously, marciorschneider (Marcio Schneider) wrote…
Yes, I don't why but without the -Modulename the mocking doesn't work with the helper module.

Yes, I see now, it's needed! It's needed because it's the module that calls the helper function, not the resource. Cool! :smiley:

Comments from Reviewable

Just three minor comments left. Then I think this looks good to me. :smiley:

Reviewed 15 of 15 files at r6.
Review status: all files reviewed, 3 unresolved discussions (waiting on @johlju and @marciorschneider)

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 29 at r5 (raw file):
Previously, marciorschneider (Marcio Schneider) wrote…
Done.

Still seeing this parameter in the comment-based help :)

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 8 at r6 (raw file):

-force

-Force (uppercase 'F')

en-US/OptionValueHelper.strings.psd1, line 1 at r5 (raw file):
Previously, marciorschneider (Marcio Schneider) wrote…
Done.

You moved the module file to /Modules/DhcpServerDsc.OptionValueHelper (very good, I like it better! :+1: ), but now the en-US folder should be moved to /Modules/DhcpServerDsc.OptionValueHelper/en-US. That way the entire module folder is standalone.
Sorry for being pain :/

Comments from Reviewable

Review status: all files reviewed, 3 unresolved discussions (waiting on @johlju and @marciorschneider)

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 29 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Still seeing this parameter in the comment-based help :)

ops, sorry

DSCResources/MSFT_DhcpPolicyOptionValue/MSFT_DhcpPolicyOptionValue.psm1, line 8 at r6 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

-force

-Force (uppercase 'F')

Ops, it was a leftover from my testing.

en-US/OptionValueHelper.strings.psd1, line 1 at r5 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
You moved the module file to /Modules/DhcpServerDsc.OptionValueHelper (very good, I like it better! :+1: ), but now the en-US folder should be moved to /Modules/DhcpServerDsc.OptionValueHelper/en-US. That way the entire module folder is standalone.
Sorry for being pain :/

Done, no problem ;)

Comments from Reviewable

Done :D

Review status: 20 of 21 files reviewed, 1 unresolved discussion (waiting on @johlju)

README.md, line 129 at r7 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should remove 'x' from the names in the change log too.

Done.

Comments from Reviewable

Reviewed 1 of 1 files at r8.
Review status: :shipit: complete! all files reviewed, all discussions resolved

Comments from Reviewable

@marciorschneider Awesome work on this! 👍 Thanks!


initial OSPP 2021 Mid-Term Itchat Puppet

Please make sure to check the following boxes creating your pull request, thank you very much!

[x] Add Blog Header, including title, author, image, categories, tags, etc.
For example:
---
title: Your Great Blog Title
author: your_github_id
categories: project
tags:
  - python
  - padlocal
image: /assets/2021/02-your-blog-slug/teaser.jpg
---

One line bio

Your blog post content Your blog post content
Your blog post content Your blog post content
Your blog post content Your blog post content

It will be easier to start drafting your post by using a existing post file as your template, for example, this one

[x] Keep all filenames lowercase, and use - to connect words instead of space. e.g. 2017-10-06-wechat-pc-impactor.md instead of 2017-10-06-WeChat PC Impactor.md

[x] Embed video/pdf from YouTube/Bilibili/local (if there's any) by using {% include iframe.html src="URL_or_PATH" %}. (Learn more about how to embed file/url in the post by reading this blog post

[x] Save photo to local before publishing, to a folder named by ${MONTH}-${YOUR_BLOG_SLUG}/ under /assets/2021/ directory.

[x] Select a beautiful and meaningful teaser image for your blog

[x] Create your contributor profile (if you are a first time contributor)
For excample:

jekyll/_contributors/lijiarui.md

---
name: Jiarui LI (李佳芮)
site: https://lijiarui.github.io
bio: 句子互动创始人 & CEO，微软人工智能最具价值专家 (AI MVP)
avatar: /jekyll/assets/contributors/lijiarui/avatar.png
email<EMAIL_ADDRESS>twitter: https://weibo.com/u/2175505900
---

Here is the example file:

lijiarui.md: https://github.com/wechaty/wechaty.js.org/blob/master/jekyll/_contributors/lijiarui.md
It will shown like this on wechaty website: https://wechaty.js.org/contributors/lijiarui/

To learn more about the contribute guideline, visit: https://wechaty.js.org/docs/contributor-program/

If it is convenient, I would like to ask you to check if there are any problems with my mid-term report
@wj-Mcat

ping @lyleshaw

ping @wj-Mcat

ping @lyleshaw

@lijiarui
Thanks to the help of @wj-Mcat , I solved a big problem and transferred the ownership to Wechaty.
Please check this PR again when you have time.
Waiting for your reply~

Please compress your PDF from 18MB to less than 3MB by using some service like https://www.ilovepdf.com/compress_pdf

Done


Update to the latest miekg/dns

What this PR does / why we need it:
Update to the latest miekg/dns which has several useful fixes
Release note:

Update the miekg/dns dependency

/assign @munnerz

/kind cleanup


add ex_encrypted and ex_kms_key_id kwargs in EC2NodeDriver.create_volume

hi @ViktorOgnev thanks for your contribution, there are only a couple of changes before we can merge, first I made a comment about explicit "truthy" checking, can you fix that on both lines please. Second the lint is failing because 3 of the lines are too long, probably the docstring comments from a glance. you can run flake8 locally to check the next commit.

hi @tonybaloney , looks like now it's ok) Should I rebase, or is it fine as is?

perfect, no need to squash, will merge.

all done, thanks for your contribution @ViktorOgnev


🎨 Modified EP into O(n^3)

Modified EP into O(n^3) with reference to the Online Appendix of Agatz et al. (2018).
Computation of T(i, j, k) has been replaced with recursion; the original code remains as footnote.

Will open another pull request after modification.

You can keep committing changes in the same branch; then they will appear in this PR automatically.


PWA-677 fixed unit test

Pull Request Test Coverage Report for Build 1536

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 37.557%

Totals

Change from base Build 1534:
0.0%

Covered Lines:
1387

Relevant Lines:
3693

💛  - Coveralls


fix: missing ngOnInit fetch data

I'm deeply sorry team, I missed that ngOnInit for Twilio to fetchData.
I've improved the e2e for the sms component so it checks that a Twilio configuration has been created and that it's indeed shown.
@hifabienne I think this PR should be reviewed as soon as you've a slot to recover the functionality
Closes #6729
Definition of Ready

[X] I am happy with the code
[X] Short description of the feature/issue is added in the pr description
[X] PR is linked to the corresponding user story
[ ] Acceptance criteria are met
[ ] All open todos and follow ups are defined in a new ticket and justified
[ ] Deviations from the acceptance criteria and design are agreed with the PO and documented.
[ ] No debug or dead code
[X] My code has no repetitions
[ ] Critical parts are tested automatically
[X] Where possible E2E tests are implemented
[ ] Documentation/examples are up-to-date
[ ] All non-functional requirements are met
[X] Functionality of the acceptance criteria is checked manually on the dev system.

Thank you @doncicuto, you are the best :tada:


Custom rule of max-len and no-use-before-define

If we want to add these rules to the eslint-config-ebates lib, we need to put these rules in the index.js file. This file is only for linting this repo...

lol. thanks for pointing out, will close and create a new PR


Things to check before enabling PHS

Many customers use an AD DS Connector account created ad-hoc and often this account does not have all the permissions required. I suggest to add a note that one must confirm that the AD DS Connector account has the required permissions for PHS:

Replicate Directory Changes
Replicate Directory Changes All
as specified here:
https://docs.microsoft.com/en-us/azure/active-directory/hybrid/reference-connect-accounts-permissions

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 3f2a7330-82fa-9c0d-0f34-b2a99ef13c64
Version Independent ID: d941f435-3443-02a1-57c0-7f4173173249
Content: Azure AD Connect: Cloud authentication via staged rollout
Content Source: articles/active-directory/hybrid/how-to-connect-staged-rollout.md
Service: active-directory
Sub-service: hybrid
GitHub Login: @billmath
Microsoft Alias: billmath

@AlexCesarini Thanks for the feedback !  I have assigned this issue to content author to investigate and update the document as appropriate.

@AlexCesarini, thanks for the suggestion! The product teams aren’t always watching this repository, so to make sure it gets in front of them for triage, please submit your idea here.
#please-close


Search review feedback

API change check
APIView has identified API level changes in this PR and created following API reviews.
azure-search-documents

/check-enforcer override


feat: add yaml adapter

Fix: https://github.com/casbin/casbin-rs/issues/213
Signed-off-by: closetool<EMAIL_ADDRESS>hi, there're some problems I can't resolve

after uncomment fixme part, compiler tells me even if I have dropped emitter

error: future cannot be sent between threads safely
  --> src/yaml_adapter.rs:37:70
   |
37 |       async fn save_policy(&mut self, m: &mut dyn Model) -> Result<()> {
   |  ______________________________________________________________________^
38 | |         if self.file_path.as_ref().as_os_str().is_empty() {
39 | |             return Err(
40 | |                 IoError::new(ErrorKind::Other, "save policy failed, file path is empty").into(),
...  |
71 | |         return Ok(());
72 | |     }
   | |_____^ future returned by `__save_policy` is not `Send`
   |
   = help: the trait `Send` is not implemented for `dyn std::fmt::Write`
note: future is not `Send` as this value is used across an await
  --> src/yaml_adapter.rs:70:9
   |
65 |         let mut emitter = YamlEmitter::new(&mut buf);
   |             ----------- has type `YamlEmitter<'_>` which is not `Send`
...
70 |         self.save_policy_file(buf).await?;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `mut emitter` maybe used later
71 |         return Ok(());
72 |     }
   |     - `mut emitter` is later dropped here
   = note: required for the cast to the object type `dyn Future<Output = std::result::Result<(), casbin::Error>> + Send`

there're some unwrap I have no idea to remove

@nlinker @mkpankov please review.

@nlinker @mkpankov please review.

@hackerchai @PsiACE
/cc @GopherJ

@hackerchai @PsiACE
/cc @GopherJ

thanks, I have improved this repo much, but there's still a piece of code confusing me
for policy in polices {
                let policy = policy
                    .into_vec()
                    .ok_or(ParsePolicyFailed("policy should be array".to_string()))?
                    .into_iter()
                    .map(|s| s.into_string().unwrap())
                    .collect();
                m.add_policy(&sec, &ptype, policy);
            }

how can I remove this unwrap which in the closure

thanks, I have improved this repo much, but there's still a piece of code confusing me
for policy in polices {
                let policy = policy
                    .into_vec()
                    .ok_or(ParsePolicyFailed("policy should be array".to_string()))?
                    .into_iter()
                    .map(|s| s.into_string().unwrap())
                    .collect();
                m.add_policy(&sec, &ptype, policy);
            }

how can I remove this unwrap which in the closure

Thanks for implementing this. Will try once outstanding code issues are resolved.

Thanks for implementing this. Will try once outstanding code issues are resolved.

Thanks guys, this is awesome!

Thanks guys, this is awesome!

after refactoring a lot of code, I hope it's better now.

add test
add ci
fix outstanding bugs
using serde-yaml instead of yaml-rust
runtime-async-std & runtime-tokio features

after refactoring a lot of code, I hope it's better now.

add test
add ci
fix outstanding bugs
using serde-yaml instead of yaml-rust
runtime-async-std & runtime-tokio features

Why use BtreeMap? There is no need to create a model for policy due to it is implemented by the casbin library.

because serde requires it
and it's not a model for policy, it's a model for yaml policy

Why use BtreeMap? There is no need to create a model for policy due to it is implemented by the casbin library.

because serde requires it
and it's not a model for policy, it's a model for yaml policy


I can't register another name.

As you can see from the screenshot, I have created three IDs for my family members. And on the first one, there is 0.29 dash for adding a second name. As I understand it, in the future, everyone will have the option to use their name in their wallet.
I would like to add another  name to my ID  9BCqJTA9Hk93yLqFUdd789xiEBSXmre3L9U8KRk4Nqg5 , but after first  registering two names with this ID, I only have 0.0952 left. This is not enough to register another name. When I try to do so, a message appears saying:
Error: Protocol error: platform deserialization error: unable to deserialize ConsensusError : UnexpectedVariant { type_name: "ConsensusError", allowed: Range { min: 0, max: 4 }, found: 65 }
Is it because there are no funds?  How can I fund my account using this ID?
I have topped up the balance at the same address
Xfy8Lq91DdSc7BPRNQ43BZxx1q99iG8Dje   (you can see it in the explorer)
where the first payment was made and now it shows 0.3 dash. However, the ID still shows -0.0952 dash.

If you are trying to register a contested username, then yes, you don't have enough funds. It costs about 0.2 Dash. Uncontested names cost around 0.0006. I guess this is the reason for the error message (we need to make this message more informative).
So it looks like you topped up your wallet receiving address in DET and next need to top up the identity using those funds so you can register a name. Good news - Identity Top Ups will be added today in release v0.5.


Rethink the caching layer

We have had a few ways of tackling the caching problem in this library and google/google-auth-library-php (#905, #925), and both of these feel like a hack.
To begin addressing this problem at its core, we should consider implementing PSR-6 interfaces in both libraries. With this, we could do one or more of the following:

Use isHit to determine if a new access token has been created
Refactor all Google/Cache classes OR remove them in favor of a standard PHP caching library

PROS: We've already written the code, refactoring shouldn't be too difficult
CONS: one more PHP caching library in the universe to be maintained

Just remove all cache classes and default cache logic, and only provide a getter/setter for users to add this.

PROS: There is not really a single lightweight caching library that i have found (maybe doctrine/cache)? So it seems best to leave this up to the users.
CONS: Some things (such as fetching certs) should always be cached, so not including a default implementation is cruel

We may also be concerned that not everyone in the community is stoked about PSR-6, although regardless of your feelings on PSR-6, it's gotta be better than using our oversimplified interface...

PSR-6! 😁 That guys bias against PSR-6 is ill founded, and is proven as such in the comments.

I am leaning towards the following implementation:

Implement PSR6
Remove CacheInterface from google/auth
Remove all files in src/Google/Cache except for the file cache
Use File Cache as the default for storing certs
That's it!

Pros:

Easy
Simplified
A default cache is provided
We aren't depending on any 3rd party caching library
Cons:
There is still no per-session caching of tokens
It's still not straightforward when a token has been received. We will need to document this.

Added in #108


[BEAM-12725] Pass null for schemaUpdateOptions

When writing temp tables during BigQuery batch loads, schema update options are invalid since the write disposition is always hard coded to WRITE_TRUNCATE. Including schema update options like this causes the load jobs to fail with the error:
"Schema update options should only be specified with WRITE_APPEND disposition, or with WRITE_TRUNCATE disposition on a table partition."

We could also address this by overriding the schema update options value for temp tables in the processElement function, similar to how its currently overriding the create and write dispositions. Let me know if that's preferred, or if I've totally missed the mark here otherwise.
R: @lukecwik
R: @chamikaramj
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

[x] Choose reviewer(s) and mention them in a comment (R: @username).
[x] Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
[ ] Update CHANGES.md with noteworthy changes.
[ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

Run Java PreCommit

Run Java_Examples_Dataflow PreCommit

Run Java_Examples_Dataflow_Java11 PreCommit

I don't think SchemaUpdateOptions can be just dropped.
See the corresponding fix to the Python side here: https://github.com/apache/beam/pull/14113

Ohh I see, so even though this small change would allow the temporary tables to be created, the copy job would still fail due to the schemas mismatching,
Okay, this is a bit chunkier than I anticipated, but I'll try to find some time to work on it.

Ah, I just came across this. Any update?

What is the next step on this PR?
@crcornwell - are you still planning to work on it? Should we close it?

Yeah I'll go ahead and close it.


Engine initialize.vi name conflicts with error ring initialize.vi

This issue was migrated from https://github.com/NIVeriStandAdd-Ons/Engine-Simulation-Toolkit-Custom-Device/issues/1
Both the RT Engine and the vi.lib "Error Ring" use a VI named "initialize.vi". This causes a conflict when building an LLB. The error ring in vi.lib uses its initialize.vi inside its GUI that is only used at edit time, so this conflict is normally not a problem because the autobuild server doesn't include edit time dependencies into its LLB builds due to the NI_AppBuilder_SDist_ExcludeEditTime=True ini key, but for others building the source on their machines it will cause a build failure.

Note: there are instructions in the README for settings the INI token.


Update Cell Profiler workflow

Don't know but the new one is valid and working.

@bgruening @beatrizserrano this was merged over a failing test, so unfortunately now new PRs are also failing. Would you mind to fix the annotation tag please?
Linting topics/imaging/tutorials/tutorial-CP/workflows/main_workflow.ga

	The 'annotation' attribute is missing or incorrect. Please add: "annotation": "<title of tutorial>"

Nevermind, Saskia tells me she fixed it in https://github.com/galaxyproject/training-material/pull/2114

Oh no, sorry! Thanks @shiltemann  @hexylena :)


LocalStorage is not checked for

Currently we uncovered a bug wherein some browser configurations without local storage support will cause the statsig client to error out. Could we get a simple check that localStorage is available in this library before accessing it?
To reproduce in Chrome, you can turn on "block all cookies"

Thank you for reporting.  We'll address that asap. cc: @tore-statsig

Ah interesting.  We've handled storage being unavailable, but it looks like the way this is implemented in chrome throws even if you are checking if local storage is defined.  We can move the try/catch up to handle this
https://github.com/statsig-io/js-client/blob/main/src/utils/StatsigLocalStorage.ts#L7

Ironically - you cant comment on a github issue if you have this setting turned on either.  I wonder how many things break across the web with this.

Yeah, a lot of things do break for sure, but we confirmed that this actually occurs fairly regularly in the wild.

And easy enough to fix - published v4.15.3 - thanks for the report!
https://github.com/statsig-io/js-client/releases/tag/v4.15.3

And thank you for the quick turnaround!


Querying from cache

Hey guys,
I have a question about how the cache query works
My scenario, I want to query the remote server only if there are no records on cache (and probably after a time strategy, that maybe anyone can help on how to handle that, but I can describe that later).
I have the following data-strategy
`
import { RequestStrategy } from '@orbit/coordinator';
export default {
create() {
return new RequestStrategy({
name: 'store-remote-query',
  source: 'store',
  on: 'beforeQuery',

  target: 'remote',
  action: 'pull',

  blocking: true,

  // Only fetch from server, if we don't have local data in cache
  //TODO the only thing I have so far in mind is how to handle the update interval for sync
  filter(query) {
    const coordinator = this.coordinator;
    const store = coordinator.getSource('store');

    let dataInCache = store.cache.query(query, query.options);
    return dataInCache.length <= 0;
  },

  catch(e) {
    this.source.requestQueue.skip();
    this.target.requestQueue.skip();

    throw e;
  }
});

},
};
`
so the idea with that is, as I described, request the server only if necessary
return this.store.liveQuery(q => q.findRelatedRecords({type: 'teacher', id: model.get('id')}, 'observations'), {       label: "Find all observations",       sources: {         remote: {           include: ['sessions', 'sessions.activities']         }       }     });
My concerns are related to this query that requires a deep level of relationships, because store.cache.query only use the query.expression not matter what options you have.
So I guess I am using the wrong method or approach
@dgeb

The strategy and filter that you've provided should only perform a remote query (or pull) if records don't exist in the cache. Is that working as you expect?
A liveQuery will only issue an initial query once and then will continue to observe changes to its associated cache. But this is passive, so if you want to force periodic remote checks, you'll want to reissue that same query periodically against your remote source and allow the results to sync with your cache.

The strategy and filter that you've provided should only perform a remote query (or pull) if records don't exist in the cache. Is that working as you expect?

Yes it is how the strategy works and if not in cache is working fine, but the issue that I have is:
teachers are in cache, observations are in cache and also sessions, but activities are not in cache. Thats why I add { include: ['sessions', 'sessions.activities'] }, but this option is not included in the cache query

@nayrban the query options you're sending ( sources: { remote: { include: ['sessions', 'sessions.activities'] } } ) are specific to the remote source. The cache itself will ignore them, and only the remote source will even be aware of them.
If the remote source is properly applying the include parameter in the query string sent to your server, then the query is working as expected.
The live query you've created should be updated whenever the cache receives an update to this particular teacher's "observations". Furthermore, as sessions and activities are updated in the cache, any model instances should be updated to reflect those updates.
Your question seems to be how to periodically update this whole graph of data from the server. You could either push changes from a web socket, or periodically poll for updates by re-issuing the query against your server.
If I'm misunderstanding, please restate what you're trying to accomplish.

thanks @dgeb , so that means, leaving the cache aside, because I now get it how it works, that if I include remote ( sources: { remote: { include: ['sessions', 'sessions.activities'] } } ) this will always make the query to the server?

@nayrban as long as the filter in your strategy is satisfied, issuing that query on the store will reach the remote source, which will query the server.
You won't need to issue a live query every time you want new data. If you're able to hold onto a reference to the live query result set, it will stay updated as results that match flow into the cache. However, you'll need to do something, like re-querying the remote source (either directly or via the store) to refresh this data.


how to determine_pause in ReplyOnPause

I cannot find the logic for detect user pause.
BTW, I want to control the period between user pause, default setting seems too sensitive
    def determine_pause(
        self, audio: np.ndarray, sampling_rate: int, state: AppState
    ) -> bool:
        """Take in the stream, determine if a pause happened"""
        duration = len(audio) / sampling_rate

        if duration >= self.algo_options.audio_chunk_duration:
            dur_vad = self.model.vad((sampling_rate, audio), self.model_options)
            logger.debug("VAD duration: %s", dur_vad)
            if (
                dur_vad > self.algo_options.started_talking_threshold
                and not state.started_talking
            ):
                state.started_talking = True
                logger.debug("Started talking")
            if state.started_talking:
                if state.stream is None:
                    state.stream = audio
                else:
                    state.stream = np.concatenate((state.stream, audio))
            state.buffer = None
            if dur_vad < self.algo_options.speech_threshold and state.started_talking:
                return True
        return False

seems algo_options.audio_chunk_duration means the time of pause, not the total audio, I'm not sure

how to make it less sensitive to detect a speech, I set speech_threshold=0.2, still get a lot of empty input

Hi @liaoweiguo - I prepared some docs on this: https://freddyaboulton.github.io/gradio-webrtc/advanced-configuration/#reply-on-pause-voice-activity-detection
audio_chunk_duration is the chunk size used to run VAD. By default its 0.6 seconds. So if you set speech_threshold=0.2, it means that if a chunk has less than 33.33% of voice activity (0.2/0.6) it will be a pause. I will set it to be lower for your case. Also you can try setting started_speaking_threshold to be higher?

this seems better:
            fn=ReplyOnPause(
                response, output_sample_rate=OUT_RATE, output_frame_size=480, algo_options=AlgoOptions(audio_chunk_duration=0.6,started_talking_threshold=0.3,speech_threshold=0.2),
            ),

Nice!


Extending / customizing a plugin

Did anyone try to extend strapi-plugin-passwordless?
I tried methods suggested in official docs and I'm not able to customize createToken service function.
I've tried the extensions folder technique and override it through server-strapi.js. Any advice on what should I try next?

@Chmarusso Also, I can try to implement the logic in the public version of the plugin. I had planned to add token patterns - numbers, symbols, etc.

@kucherenko
That's a tutorial that I tried to follow:
https://docs.strapi.io/dev-docs/plugins-extension#extending-a-plugin-s-interface

I had planned to add token patterns - numbers, symbols, etc.
That is exactly the reason why I'm trying to customize your extension 😃
I would like to use customAlphabet from nanoid and just generate tokens that are only containing 0-9

In the meantime, I managed to override plugin in the following way:

I've copied your package directory to my root dir of strapi
changed nanoid call to use customAlphabet
installed package from local directory npm install ./path/strapi-plugin-passwordless

Not ideal, but works.
Regarding how your plugin may support custom alphabets in future. Perhaps something like this:
    async createToken(email, context) {
      const settings = await this.settings();
      const {token_length = 20, alphabet = "-_1234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"} = settings;
      await strapi.query('plugin::passwordless.token').update({where: {email}, data: {is_active: false}});
      const body = customAlphabet(alphabet, token_length);
      const tokenInfo = {
        email,
        body: body(),
        context: JSON.stringify(context)
      };
      return strapi.query('plugin::passwordless.token').create({data: tokenInfo});
    },

@kucherenko
Hi,
is there any progress there.
I plan to generate a code that only contains numbers.
Are there already settings for this or do you have to overwrite the code yourself?

Hi! I could do this, it may help:

Create a folder named passwordless in src/extensions
Create a strapi-server.ts file with the following:

import passwordlessService from "./services/passwordless";

export default (plugin) => {
  plugin.services.passwordless = passwordlessService;

  return plugin;
};

Use this function to replace a controller or service as you wish. In my case replaces the passwordless service with a custom service.

Ok

Hi! I could do this, it may help:

Create a folder named passwordless in src/extensions
Create a strapi-server.ts file with the following:

import passwordlessService from "./services/passwordless";

export default (plugin) => {
  plugin.services.passwordless = passwordlessService;

  return plugin;
};

Use this function to replace a controller or service as you wish. In my case replaces the passwordless service with a custom service.

Okay thanks i will try it

Thanks @marcosnc08
Here is a complete working implementation.
import { Strapi } from '@strapi/strapi'
import passwordlessService from 'strapi-plugin-passwordless/server/services/passwordless'

const extendedPasswordlessService = ({ strapi }: { strapi: Strapi }) => {
  const { sendLoginLink: _extracted, ...otherMethods } = passwordlessService({ strapi })

  return {
    ...otherMethods,
    async sendLoginLink(token) {
      const settings = await this.settings()
      const user = await this.fetchUser({ email: token.email })
      let context = undefined

      try {
        context = JSON.parse(JSON.parse(token.context))
      } catch (_) {
        console.error('passwordless send-link failed to parse context: context should be in JSON format.')
      }

      const params = {
        URL: settings.confirmationUrl,
        CODE: token.body,
        USER: user,
        CONTEXT: context,
      }

      console.log('sendLoginLink params:', params)

      const text = await this.template(settings.message_text, params)
      const html = await this.template(settings.message_html, params)
      const subject = await this.template(settings.object, params)

      const sendData = {
        to: token.email,
        from: settings.from_email && settings.from_name ? `${settings.from_name} <${settings.from_email}>` : undefined,
        replyTo: settings.response_email,
        subject,
        text,
        html,
      }
      // Send an email to the user.
      return await strapi.plugin('email').service('email').send(sendData)
    },
  }
}

export default (plugin) => {
  plugin.services.passwordless = extendedPasswordlessService

  return plugin
}

And here is the log I get:
sendLoginLink params: {
  URL: 'http://localhost:3000/fr/mon-compte',
  CODE: 'k3D2mMGKZqkqv4gCkUmq',
  USER: {
    id: 18,
    username: 'Kim',
    email<EMAIL_ADDRESS>    provider: null,
    confirmed: false,
    blocked: false,
    createdAt: '2024-02-01T11:20:58.478Z',
    updatedAt: '2024-02-01T11:20:58.478Z',
    role: {
      id: 6,
      name: 'Authenticated',
      description: 'Default role given to authenticated user.',
      type: 'authenticated',
      createdAt: '2023-03-02T18:53:41.863Z',
      updatedAt: '2023-03-02T18:53:41.863Z'
    }
  },
  CONTEXT: { to: 'http://localhost:3000/fr/ici' }
}

And here is how it can be used in the admin "Subject", "Message (text)" and "Message (html)" fields:
Context.to: <%= CONTEXT.to %>
USER.username: <%= USER.username %>

I saw in the code that the templates use lodash template, where we can find more details about syntax.

thank @cvolant for the code exemple, I was able to change some parts and use an email template from Brevo directly, instead of the standard template of passwordless plugin

@cvolant I just realized there's a double parse on your code
context = JSON.parse(JSON.parse(token.context))

Hi, thank you for the issue.
To be honest I didn't try to rewrite services, could you please give me the link to the official docs that are you trying to follow?

@kucherenko hey~may i ask is there any progress there? still have a plan on this ?

hey @ggggaolf, I'm sorry I don't have time for implementing the feature right now, will try to do that in few months


recover CI

https://github.com/ks6088ts-labs/baseline-environment-on-azure-terraform/actions/runs/11584919203

https://github.com/ks6088ts-labs/baseline-environment-on-azure-terraform/commit/336f31388de60e83039d1a65b2d15210016f1e5b


docs: eks cluster upgrade instructions

PR o'clock
Description
added eks cluster upgrade instructions into the FAQ
Checklist

[ ] CI tests are passing
[ ] README.md has been updated after any changes to variables and outputs. See https://github.com/terraform-aws-modules/terraform-aws-eks/#doc-generation

First part of this instruction  :+1: is from me, I also learn about kubegrunt whcih I didnt know,  but have some mixed feelings about examples from kubegrunt. Main concern is that if we put this in official module repo then this can cause that community will be addressing issues caused by that tool to this repo.
@barryib what are your feelings here?