Fix xDnsServerADZone cim session bug

This is an attempt to resolve #53 .  I've done the following:

New up a cim session only when computername is passed in as a parameter.

This logic is duplicated in Get-TargetResource & Set-TargetResource.  Is there a cleaner way?

Throw an exception in Get-TargetResource when Credentials are passed in but no ComputerName is passed since Passing Credentials without a computername makes no sense.

What is the correct way of throwing a dsc exception because of invalid use of parameters?
Do i need to do this same validation in the Set-TargetResource function?

Anytime a new-cimsession is created be sure to cleanup with a call to remove-cimsession
Added tests to verify the above statements

Is there a cleaner way to write the tests?

/cc @johlju

This change is

Also is there some sort of "public chat" in which i can get faster feedback from the community on any questions i have on DSC resource development.  I'm on the powershell.slack.com #dsc channel but there isn't much communication going on in regards to "Contributing" to these DSC modules.

Please look at the PowerShell\DscResource Gitter channel. There are not a lot of activity there, but if we get more people there it might be. Hopefully you will get your questions answered here.

I will answer the other questions in the PR description when I have an opportunity :smile:

FYI I have not performed any actually testing within a vm. I’ll post back my results when I do

To answer the above questions

This logic is duplicated in Get-TargetResource & Set-TargetResource. Is there a cleaner way?

You could create a helper function in the same module file as the *-TargetResource function, a helper function that creates a CimSession and returns an CimSession object. Maybe you could move some of the error handling to that as well then.

What is the correct way of throwing a dsc exception because of invalid use of parameters?

Not entirely sure what you looking for here. But in other resource modules we have a common helper function New-InvalidArgumentException; see CommonResourceHelper.psm1. NBut that is not an requirement. Just throwing an error saying that wrong parameters was provided will suffice.

Do i need to do this same validation in the Set-TargetResource function?

It wouldn't hurt, but if you have the error checking in Test-TargetResource I think that would be enough. Personally I try to use the same error handling in all *-TargetResource functions if one day either can be called individually.

Is there a cleaner way to write the tests?

At a quick glance the tests looks great with "What-When-Should" test descriptions! Great work on the tests! You should skip -MockWith { } when you are not mocking any code, and remove ';' from the end of the lines. When splatting, add all the commands to the splatting variable, shorter lines is easier for the reviewer. But that is just style changes.

Review status: 0 of 2 files reviewed at latest revision, 12 unresolved discussions.

a discussion (no related file):
It hasn't anything to do with your change, but if you would like it would be great if we could resolve these PSSA warnings
https://ci.appveyor.com/project/PowerShell/xdnsserver/build/<IP_ADDRESS>#L601

DSCResources/MSFT_xDnsServerADZone/MSFT_xDnsServerADZone.psm1, line 59 at r2 (raw file):

    Write-Verbose ($LocalizedData.CheckingZoneMessage -f $Name, $Ensure)

    if (!$PSBoundParameters.ContainsKey('ComputerName') -and $PSBoundParameters.ContainsKey('Credential'))

I think a helper function would help here. Moving line 59-62 and 69-84 into a helper function would make there error handling work for both Get-TargetResource and Set-TargetResource (Test-TargetResource will throw when calling the Get-TargetResource).
It would also be less code in the *-TargetResource function, and also less code in the tests since you could test the helper function once that it throws. Then you could (optionally) mock the helper function when testing the *-TargetResource functions.

DSCResources/MSFT_xDnsServerADZone/MSFT_xDnsServerADZone.psm1, line 61 at r2 (raw file):

    if (!$PSBoundParameters.ContainsKey('ComputerName') -and $PSBoundParameters.ContainsKey('Credential'))
    {
        throw "The Credentials Parameter can only be used when ComputerName is also specified"

We should this a localized string to be consistent with he other strings that are localized.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):

            }

            Context 'When a computername is not passed' {

In this Context-block I suggest we add a BeforeAll-block and add both the mocks inside it. That way all calls are mocked if the test fails and the code is actually run then the mocks will catch the calls and not calling the real functions.
Context 'When a computername is not passed' {
    BeforeAll {
        Mock -CommandName New-CimSession
        Mock -CommandName Remove-CimSession
    }

    ...

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):

computername

I think this should be 'computer name', instead of the parameter name. Throughout.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 99 at r2 (raw file):

                It 'Should not call New-CimSession' {
                    Mock -CommandName New-CimSession
                    Get-TargetResource @testParams -ReplicationScope $testReplicationScope

I think we should test the call to Get-TargetResource as well.
{ Get-TargetResource @testParams -ReplicationScope $testReplicationScope } | Should -Not -Throw

Throughout were applicable.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 114 at r2 (raw file):

                            Credential = $testCredential
                        }
                        { Get-TargetResource @withCredentialsParameter -ReplicationScope $testReplicationScope } | Should -Throw

We should make sure the correct error message is returned here. Not just any error.
... | Should -Throw 'error message'

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 125 at r2 (raw file):

                    }
                    Mock -CommandName New-CimSession
                    Get-TargetResource @withComputerNameParameter -ReplicationScope $testReplicationScope

Won't this also call Remove-CimSession? Should we Mock that? We don't need to assert it here since it is asserted below.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 132 at r2 (raw file):

                        ComputerName = $testComputerName
                    }
                    Mock -CommandName New-CimSession -MockWith { New-MockObject -Type Microsoft.Management.Infrastructure.CimSession }

How come this is mocked using an object, but the previous It-block wasn't?

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 143 at r2 (raw file):

                            Credential = $testCredential
                        }
                        Mock -CommandName New-CimSession

Should we mock Remove-CimSession here as well?

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 263 at r2 (raw file):

            }

            Context 'When a computername is not passed' {

Could you look over these It-blocks below and see if any of the previous comments apply here as well?

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 300 at r2 (raw file):

                }
                Context 'When credentials are passed' {
                    It 'Should throw an exception indicating a computername must also be passed' {

This tests does not test for an error being thrown as the description says.

Comments from Reviewable

Reviewed 2 of 2 files at r1.
Review status: 1 of 2 files reviewed at latest revision, 12 unresolved discussions.

Comments from Reviewable

Hey @johlju , I', not really sure how to fix some of these
https://ci.appveyor.com/project/PowerShell/xdnsserver/build/<IP_ADDRESS>#L338
Along with the BOM ones.

Just now resolved all the psscriptanalyzer issues!

Hmm, it also looks like me rewriting history causes your line and file specific review to now be out of sync with the current history.  sorry about that.  I should be able to figure out which lines u were referring to based on the context you provide in the review comments.

Review status: 0 of 15 files reviewed at latest revision, 12 unresolved discussions.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
In this Context-block I suggest we add a BeforeAll-block and add both the mocks inside it. That way all calls are mocked if the test fails and the code is actually run then the mocks will catch the calls and not calling the real functions.
Context 'When a computername is not passed' {
    BeforeAll {
        Mock -CommandName New-CimSession
        Mock -CommandName Remove-CimSession
    }

    ...

How about this instead to try up the tests.
        Context 'When a computername is not passed' {
            Mock -CommandName New-CimSession
            Mock -CommandName Remove-CimSession
            Mock -CommandName Get-TargetResource -MockWith { return $fakePresentTargetResource }
            Set-TargetResource @testParams -ReplicationScope $testReplicationScope

            It 'Should not call New-CimSession' {
                Assert-MockCalled -CommandName New-CimSession -Scope It -Times 0 -Exactly
            }

            It 'Should not call Remove-CimSession' {
                Assert-MockCalled -CommandName Remove-CimSession -Scope It -Times 0 -Exactly
            }
        }

Comments from Reviewable

.MetaTestOptIn.json, line 1 at r3 (raw file):

[

Not sure if i need this file.  Couldn't find any good documentation on this.  All i know is that is gets rid of alot of BOM issues etc.  I just stole this content from the SqlServerDSC module

Comments from Reviewable

a discussion (no related file):
Previously, johlju (Johan Ljunggren) wrote…
It hasn't anything to do with your change, but if you would like it would be great if we could resolve these PSSA warnings
https://ci.appveyor.com/project/PowerShell/xdnsserver/build/<IP_ADDRESS>#L601

Done as long as i did the .MetaTestOptIn.json file thing right.

Comments from Reviewable

@TraGicCode The file .MetaTestOptIn.json is used to opt-in for common tests that might break testing until the code is aligned to the style guideline. Awesome work opting in for these tests! You can read a little about it here, but there isn't much more information around it (I realized now) https://github.com/PowerShell/DscResource.Tests#phased-meta-test-opt-in
I will review!

Reviewed 1 of 1 files at r2, 14 of 14 files at r3.
Review status: all files reviewed at latest revision, 15 unresolved discussions.

a discussion (no related file):
Previously, TraGicCode (Michael Fyffe) wrote…
Done as long as i did the .MetaTestOptIn.json file thing right.

Awesome work fixing all the PSSA warnings for the entire module :smile:  I bow and thank you! :bow:  One big step closer to HQRM.

a discussion (no related file):
Please add what this change does to the Unreleased section in the README.md. Also please add that all PSSA rule warnings was resolved as a separate row under the Unreleased section.

.MetaTestOptIn.json, line 1 at r3 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Not sure if i need this file.  Couldn't find any good documentation on this.  All i know is that is gets rid of alot of BOM issues etc.  I just stole this content from the SqlServerDSC module

LGTM. Awesome work adding this!

DSCResources/MSFT_xDnsServerSetting/MSFT_xDnsServerSetting.psm1, line 254 at r3 (raw file):

function Test-TargetResource
{
    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSDSCUseVerboseMessageInDSCResource", "")]

Instead of suppressing this, I suggest to add a 'Write-Verbose' call to this function.
Something like this maybe?
Write-Verbose -Message 'Evaluating the DNS server settings.'

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
How about this instead to try up the tests.
        Context 'When a computername is not passed' {
            Mock -CommandName New-CimSession
            Mock -CommandName Remove-CimSession
            Mock -CommandName Get-TargetResource -MockWith { return $fakePresentTargetResource }
            Set-TargetResource @testParams -ReplicationScope $testReplicationScope

            It 'Should not call New-CimSession' {
                Assert-MockCalled -CommandName New-CimSession -Scope It -Times 0 -Exactly
            }

            It 'Should not call Remove-CimSession' {
                Assert-MockCalled -CommandName Remove-CimSession -Scope It -Times 0 -Exactly
            }
        }

Best practice (or what we should call it) is to call the function that is being tested inside the It-block. Pester has all the testing logic around the It-block. Context-block is just a logic "placeholder" to mock things where BeforeAll-, BeforeEach-, AfterAll-, AfterEach-block can be used to help with mocking. It is okay to keep the mocks in the Context block without using any Before*-block.

Comments from Reviewable

a discussion (no related file):
Previously, johlju (Johan Ljunggren) wrote…
Awesome work fixing all the PSSA warnings for the entire module :smile:  I bow and thank you! :bow:  One big step closer to HQRM.

Thank! :thumbsup:

Comments from Reviewable

a discussion (no related file):
Previously, johlju (Johan Ljunggren) wrote…
Please add what this change does to the Unreleased section in the README.md. Also please add that all PSSA rule warnings was resolved as a separate row under the Unreleased section.

Done.

Comments from Reviewable

.MetaTestOptIn.json, line 1 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
LGTM. Awesome work adding this!

Good to hear!

Comments from Reviewable

DSCResources/MSFT_xDnsServerADZone/MSFT_xDnsServerADZone.psm1, line 61 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should this a localized string to be consistent with he other strings that are localized.

Done.

Comments from Reviewable

DSCResources/MSFT_xDnsServerSetting/MSFT_xDnsServerSetting.psm1, line 254 at r3 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Instead of suppressing this, I suggest to add a 'Write-Verbose' call to this function.
Something like this maybe?
Write-Verbose -Message 'Evaluating the DNS server settings.'

Done.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Best practice (or what we should call it) is to call the function that is being tested inside the It-block. Pester has all the testing logic around the It-block. Context-block is just a logic "placeholder" to mock things where BeforeAll-, BeforeEach-, AfterAll-, AfterEach-block can be used to help with mocking. It is okay to keep the mocks in the Context block without using any Before*-block.

Hmm, Okay i can do this.  Not sure i agree with the "Best Practice".  In rspec it's common to do this to reduce duplication by performing the operation on the SUT and making multiple assertions about the SUT instead.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Hmm, Okay i can do this.  Not sure i agree with the "Best Practice".  In rspec it's common to do this to reduce duplication by performing the operation on the SUT and making multiple assertions about the SUT instead.

Done.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…

computername

I think this should be 'computer name', instead of the parameter name. Throughout.

Done.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 99 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
I think we should test the call to Get-TargetResource as well.
{ Get-TargetResource @testParams -ReplicationScope $testReplicationScope } | Should -Not -Throw

Throughout were applicable.

Hmm...can not sure where you wanting this to go.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 132 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
How come this is mocked using an object, but the previous It-block wasn't?

I was running into an exception from Remove-CimSession but i think, as per your previous comments, that it was because i missed a mock to remove-cimsession.  I will see if i can get rid of these now.

Comments from Reviewable

Reviewed 1 of 14 files at r3.
Review status: 11 of 15 files reviewed at latest revision, 15 unresolved discussions.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 114 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should make sure the correct error message is returned here. Not just any error.
... | Should -Throw 'error message'

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 125 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Won't this also call Remove-CimSession? Should we Mock that? We don't need to assert it here since it is asserted below.

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 143 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should we mock Remove-CimSession here as well?

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 263 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Could you look over these It-blocks below and see if any of the previous comments apply here as well?

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 300 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
This tests does not test for an error being thrown as the description says.

Done.

Comments from Reviewable

I have confirmed that this does indeed fix the ability to manage xDnsServerAdZones on a local node.

I will continue the review later tonight.

Review status: 11 of 15 files reviewed at latest revision, 15 unresolved discussions.

a discussion (no related file):
Previously, TraGicCode (Michael Fyffe) wrote…
Thank! :thumbsup:

LGTM. if you can't acknowledge this review comment, could you write 'Done' on this one and I will see if I can  acknowledge it after that. Wish it existed an 'Force resolve' button.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Done.

LGTM. I see your point, but maybe Pester is not that intelligent yet? I throw together an example in a gist here; https://gist.github.com/johlju/0767c9ebde5fffb22335cfa237aa96b2
It shows that the It-block's is not being called if an unexpected error occurs. Although that is a problem, the last example illustrates where one of the code path is actually being called successfully.
If there is a better way, I am eager to learn! :smile:
Either way, the code changes for this comment look good to me! :smile:

Comments from Reviewable

Review status: 11 of 15 files reviewed at latest revision, 14 unresolved discussions.

a discussion (no related file):
Previously, johlju (Johan Ljunggren) wrote…
LGTM. if you can't acknowledge this review comment, could you write 'Done' on this one and I will see if I can  acknowledge it after that. Wish it existed an 'Force resolve' button.

Done

Comments from Reviewable

Review status: 11 of 15 files reviewed at latest revision, 13 unresolved discussions.

.MetaTestOptIn.json, line 1 at r3 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Good to hear!

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
LGTM. I see your point, but maybe Pester is not that intelligent yet? I throw together an example in a gist here; https://gist.github.com/johlju/0767c9ebde5fffb22335cfa237aa96b2
It shows that the It-block's is not being called if an unexpected error occurs. Although that is a problem, the last example illustrates where one of the code path is actually being called successfully.
If there is a better way, I am eager to learn! :smile:
Either way, the code changes for this comment look good to me! :smile:

Done.

Comments from Reviewable

DSCResources/MSFT_xDnsServerADZone/MSFT_xDnsServerADZone.psm1, line 59 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
I think a helper function would help here. Moving line 59-62 and 69-84 into a helper function would make there error handling work for both Get-TargetResource and Set-TargetResource (Test-TargetResource will throw when calling the Get-TargetResource).
It would also be less code in the *-TargetResource function, and also less code in the tests since you could test the helper function once that it throws. Then you could (optionally) mock the helper function when testing the *-TargetResource functions.

Still haven’t done this. If u wanna take a stab feel free too.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 96 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Done.

Done.

Comments from Reviewable

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 99 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Hmm...can not sure where you wanting this to go.

If you wanna take a stab here as well feel free.

Comments from Reviewable

Reviewed 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

DSCResources/MSFT_xDnsServerADZone/MSFT_xDnsServerADZone.psm1, line 59 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
Still haven’t done this. If u wanna take a stab feel free too.

Wish I could help, but currently I haven't got the time :/ I spend the time I can reviewing. :smile:
Let's leave it as is. Write done on this and I will acknowledge this as resolved.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 99 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
If you wanna take a stab here as well feel free.

Same as previous comment. Wish I had the time. Let's leave it as-is for now.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 132 at r2 (raw file):
Previously, TraGicCode (Michael Fyffe) wrote…
I was running into an exception from Remove-CimSession but i think, as per your previous comments, that it was because i missed a mock to remove-cimsession.  I will see if i can get rid of these now.

I couldn't acknowledge this one. Try writing 'Done' on this one and I will see if I can acknowledge it.

Comments from Reviewable

Reviewed 1 of 4 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

DSCResources/MSFT_xDnsServerADZone/MSFT_xDnsServerADZone.psm1, line 59 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Wish I could help, but currently I haven't got the time :/ I spend the time I can reviewing. :smile:
Let's leave it as is. Write done on this and I will acknowledge this as resolved.

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 99 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Same as previous comment. Wish I had the time. Let's leave it as-is for now.

Done.

Tests/Unit/MSFT_xDnsServerADZone.Tests.ps1, line 132 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
I couldn't acknowledge this one. Try writing 'Done' on this one and I will see if I can acknowledge it.

Done.

Comments from Reviewable

@johlju I've marked the remaining discussions as resolved as you mentioned.

Let me know if there is anything else on my end needed in order to get this past the finish line.

Just a 2 more comments and after that I think it looks good to me. :smile:

Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

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

    * If you do not specify this parameter, the command runs on the local system.
* **Credential**: Specifies the credential to use to create the AD zone.
  * If you do not specify this parameter, the command runs as the local system.

Please add to the parameter description that this parameter can only be used at the same time as parameter ComputerName.

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

...preventing AD Integrated Zones from happening.

Could you please rephrase this, and make more of a summary of the problem in issue #53 that is resolved by this PR? For example that ComputerName is no longer needed, and in when CimSession is used and not used, and when Credential parameter is needed. Notable changes that the users should be aware of.

Comments from Reviewable

Review status: 14 of 15 files reviewed at latest revision, 2 unresolved discussions.

README.md, line 44 at r4 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Please add to the parameter description that this parameter can only be used at the same time as parameter ComputerName.

Done.

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

...preventing AD Integrated Zones from happening.

Could you please rephrase this, and make more of a summary of the problem in issue #53 that is resolved by this PR? For example that ComputerName is no longer needed, and in when CimSession is used and not used, and when Credential parameter is needed. Notable changes that the users should be aware of.

Done.

Comments from Reviewable

Let me know if this is good enough :bow:

Review status: 14 of 15 files reviewed at latest revision, 2 unresolved discussions.

Comments from Reviewable

@kwirkykat or @mbreakey3 can you take a glance at this and merge when you got a chance?

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.

Comments from Reviewable

@TraGicCode Ah man, I was to quick again... 😞 Can you fix the lint errors here https://ci.appveyor.com/project/PowerShell/xdnsserver/build/<IP_ADDRESS>#L626

Yep i'm on it.  Was going to handle it this morning.  Builds take so long i tend to wander off and come back later to check on things :(

Yes no worries, normally I check the "checks" before giving LGTM, but recently I have forgotten that :smile:

This time it's really LGTM. :smile:

Reviewed 1 of 1 files at r6.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

@kwirkykat or @mbreakey3 can you take a glance at this and merge when you got a chance?
Pinging again since the PR wasn't really ready on the last ping.


bumping js-slang to fix indentation in CSE machine

Pull Request Test Coverage Report for Build<PHONE_NUMBER>
Details

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 33.098%

Totals

Change from base Build<PHONE_NUMBER>:
0.0%

Covered Lines:
5288

Relevant Lines:
14861

💛  - Coveralls


shiva can pry bolted doors

Description
wtf real

Unfathomably based. Anyways that's because Shiva inherits from the spider proto which has the prying component with pryPowered(pry powered doors) and force(pry bolted doors) bools to true.

See #23449

Should this issue be closed?

Duplicate of https://github.com/space-wizards/space-station-14/issues/30157


When running the Prisma2 tutorial

When trying to download the demo script as part of the Prisma2 tutorial init flow, I get this error:
Preparing your demo script (TypeScript) ...

✔  Downloading the demo script from GitHub ...
✔  Extracting content to hello-prisma2 ...
⠙  Installing dependencies with: `npm install` ...
4  Preparing your database ...
 ERROR  Error during command execution

Error: Could not find /Users/etel/Documents/code/test_nikos_article/hello-prisma2/prisma/project.prisma
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! script@ postinstall: `prisma2 generate`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the script@ postinstall script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR!      /Users/etel/.npm/_logs/2019-09-09T13_15_08_063Z-debug.log

Running npm install manually does not result in an error.

I can't reproduce this, can you please list out the exact step that you took in the init flow.
Also, there is a very similar issue to this one: https://github.com/prisma/prisma2/issues/482. So this might be a duplicate.

Which prisma2 version are you using @etelsv
Given it says project.prisma I suspect this to be an older version!

This is not reproducible anymore, close this one. Please feel free to open a new issue in case you still run into it and tag me there.


@internal is not stripped out during docs auto-generation

see https://github.com/zwave-js/node-zwave-js/pull/1475

resolved in 313b187f94c0fd22a300c0fc4eb213e69c4f014a

resolved in 313b187f94c0fd22a300c0fc4eb213e69c4f014a


Issue with custom CA certificates (was: Posting comments doesn't work with '404 not found')

Checklist

[ ] I am running the latest version. Installing Isso from GitHub from the master branch does not fix my issue — I am using the latest docker image: ghcr.io/isso-comments/isso:0.13.0
[x] I have checked the troubleshooting guide
[x] I have searched the open issues, but my issue has not already been reported

What is not working?
I have ISSO showing imported comments, but posting comments doesn't work.
When I try to post a comment, a POST is sent to https://comments.domain.tld/new?uri=%2F2019%2F12%2Fblog-title-slug%2F and ISSO returns 404:
<!doctype html>

<title>404 Not Found</title>

Not Found

URI does not exist %s

How can one reproduce this issue?
Here is my isso.cfg:
[general]
name = domain
host =
  http://blog.domain.tld/
  https://blog.domain.tld/
dbpath = /config/comments.db
gravatar = true

[guard]
enabled = true
ratelimit = 2

Here is my nginx config for comments.domain.tld:
server {
  server_name comments.domain.tld;
  listen 443 ssl;
  listen 80;
  ssl_certificate /etc/letsencrypt/live/comments.domain.tld/fullchain.pem;
  ssl_certificate_key /etc/letsencrypt/live/comments.domain.tld/privkey.pem;
  location / {
    proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
    proxy_set_header Host $host;
    proxy_set_header X-Forwarded-Proto $scheme;
    proxy_pass http://isso:8081;
  }
}

Here is the HTML that's used to load the comments on the page:

                  Javascript needs to be activated to view comments.

ISSO is being run using the docker image ghcr.io/isso-comments/isso:0.13.0, with the default command which I believe is this (only modified the port):
/isso/bin/gunicorn -b <IP_ADDRESS>:8081 -w 4 --preload isso.run --worker-tmp-dir /dev/shm

OK - here's something super weird: I can post comments to posts that already have comments thanks to the wordpress import tool.
But on any posts that have zero comments, it doesn't work.

OK - figured it out!
This is unique to my setup, and perhaps this can help someone else who has a similar setup: the problem is related to this line (and btw, there's a bug here because %s is printed instead of the URI): https://github.com/posativ/isso/blob/13bcfbf41d92eebadfdf78db96dfbff6ada15095/isso/views/comments.py#L305
The issue is that I was using a local setup with a local root CA to server HTTPS locally so that I could access the site locally under HTTPS. However, when ISSO tries to fetch the post title by accessing the website via origin, it fails because it's accessing it via HTTPS, and the docker container isn't told to trust the local root CA I'm using.
This problem is solved by copying in the root certificate into /usr/local/share/ca-certificates/ and running update-ca-certificates inside the container.
It would be nice though if this weird roundabout query wasn't necessary though - if instead the title could be passed in along with the original comment somehow.

It would be nice though if this weird roundabout query wasn't necessary though - if instead the title could be passed in along with the original comment somehow, then this wouldn't be necessary.

Ah ha! This is possible! 😄

Furthermore you can override the automatic title detection inside the embed tag, as well as the thread ID, e.g.:

Now having done that everything works and there's no need to copy the CA cert in or access the page over HTTP for it work locally!
Perhaps this issue could be turned into a request for improving the error that's reported by the server?
There's already a bug in the error message (the %s thing), but what would be even better is if the error message recommended users try setting data-title and data-isso-id per the client configuration docs.

Improving the error handling is on Isso's TODO list, see https://github.com/posativ/isso/issues/673
You're most welcome to submit a PR to make this issue more clear (for now, at least on the JS console.log)

@taoeffect Thank you so much for this workaround!! Had many painful hours of debugging already and thanks to your finding I can sleep again :)

@finga no problem, I'm glad it was helpful to a follower of the Sacred Chao.


fix: use String.Chars.t() instead of binary() for put_toast message

The message does not have to be a binary, but needs to implement String.Chars for it to be rendered. This allows structs to be used like ErrorMessage without having to call to_string(error).

I like it. Thanks!


Commit message triggers autosave when no other files open

Repro:

Open a project
Open a file, make changes
Close the file
Add All, Commit, and Push via git-plus
Type a commit message
Save

The push is successful, but the autosave dialog is triggered on the commit message editor. This doesn't happen if you have at least one file open when you Add/Commit/Push (presumably since the commit message then opens in a new pane).

What is the autosave dialog? May be this is similar to #235.

Is this related to the auto-save package? Maybe try disabling that feature and see if this still happens. I've opened a pull request to that package to allow other packages to disable it on a per-file basis but nothing has changed yet.

I don't have autosave enabled.

Yea I haven't seen this happen yet.
I have a new branch called 5.5.0 in this repo which is going to be the next release. You can clone that and replace your installation of git-plus with it and try it out to see if this still happens

Looks to be fixed on 5.5.0. Thanks!

Can someone close this issue now? :)


Improve configuration docs, add setServerName method for parity with …

…other options.

Thank you so much! :-) Super fast response too, kudos.

👍


Sswarn/skip

Added skip, skiplast, and skipwhile.

Question: Does it make sense to have a channel version of  skipLast? not sure how we figure out how many elements are in a channel without removing all elements from a channel

Let's not implement a channel version of SkipLast at the moment. If we decide that's useful in the future we can come back to it, but it doesn't seem like the best use of time.


lc3tools: init at 0.12

Motivation for this change
Added the lc3tools toolchain and emulator for the LC-3 architecture.
Things done

[ ] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)

[X] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
[X] Tested execution of all binary files (usually in ./result/bin/)
[ ] Determined the impact on package closure size (by running nix path-info -S before and after)
[ ] Ensured that relevant documentation is up to date
[X] Fits CONTRIBUTING.md.

I think ofborg is broken?

I think ofborg is broken?

Try rebasing on master to catch https://github.com/NixOS/nixpkgs/commit/f75c8753d80959f6bf63237748e799849c74179f

I think ofborg is broken?

Try rebasing on master to catch f75c875

@SuperSandro2000 Done

...still seems broken

This is a regression, someone pushed a change that broke evaluation.

@GrahamcOfBorg eval

@GrahamcOfBorg eval

rebased and checks pass now!

While I'm actually fine with adding the package itself, please make sure that the executables are installed into $out/bin.

Done.


Charset directive in content-type header is not supported

According to https://www.rfc-editor.org/rfc/rfc9110.html#name-charset a content-type header might include a charset directive.
If I send a request with a content-type header that includes a charset directive, it responds with BadRequest.
It works properly without the charset directive.
Request
curl --location --request POST 'http://localhost:4000' \
--header 'Apollo-Require-Preflight: 1' \
--header 'Content-Type: application/json; charset=utf-8' \
--data-raw '{"query":"...",}'

Response:
{
  "errors": [
    {
      "message": "POST body missing, invalid Content-Type, or JSON object has no keys.",
      "extensions": {
        "code": "BAD_REQUEST"
      }
    }
  ]
}

Fix snuck in here #57 !

@BlenderDude could you publish a patch release to npm please? I am also hitting this with requests coming from a .NET-based HttpClient.

@coyoteecd Done!


#1143 Helm issues while deploying using argocd

This PR fixes some helm issues while deploying using argocd, namely a CRD which doesn't pass validation, and make the webhook-cleanup-job annotations configurable
This should close issue #1143

@googlebot I signed it!

@googlebot I signed it!


A letter to the EOS holders

A letter to the EOS holders
Dear all helpless EOS holders,
Do you still have confidence in EOS? Do you still be patient with EOS and Block.one?  We present you SPEECH!
In SPEECH, you can say anything you want, we forward every words you said to b1. And meanwhile, your speech is golden, every time you make a speech,  you earn some SPEECH coins. More words, more coin. More saying, more coin.
Oh, one more thing: In past few days, first it's CPU mining, then RAM mining, it makes you can't stop wondering: why not NET? NET is the cheapest resource between these three. How about make some Roast of EOS/B1, and meantime using NET to mine some coin? This is natural, right?
-- Team of speech
So here comes the rules:

Everyone can say something, it's a free blockchain world.
We can think this as a warm up of VOICE.
More words, more coin, coin doubled when words is multiple of 500 byte, no upper bound.
If speech time is continuous (you don't miss every block), more coin.
You can only make one speech in one block.
Every transaction, miner get 70% coin, contract get 30% coin.
Every speech miner says, we forward it to b1.
Total supply of SPEECH is 100 million, just like B1's tokens in EOS.
The reward start from 4 SPEECH each block, decrease progressively every 5 million, the decrease factor is 1.2.
You can making speech starts from UTC 2019-12-07 12:00:00.

How to make speech, and earn SPEECH:

Wait for UTC 2019-12-07 12:00:00.

Just say words start with Hi Block.one:.

cleos push action speechforeos say '["my_account", "Hi Block.one:I WANT TO SAY THAT..."]' -p my_account

Earned SPEECH coin, more words is encouraged.

from: https://github.com/cantstopwondering/speech

i think b1 want to delete your contract...

Github issues are not an announcement page.

Funny project!
I just looked into b1 account, wah, that's a lot of F* words..
It looks like someone really have something to say to b1.
https://bloks.io/account/b1


Leading spaces not being removed on Windows

There's an issue with leading spaces that #12 is an attempt to fix.
@strangepete Can you provide the specific text that's causing the issue? I'd like to keep the bug report separate from the potential PR if that makes sense.

Sure, and sorry for not filing an issue first, a little over confident hah
Windows 10
Janet 1.37.1
using documentarian installed via jpm install [git url]
This is the rendered output I get when the template has two leading spaces. The sections that follow a markdown list with two spaces are interpreted as being a multiline list item, and are indented as such:

spi/read-opt
cfunction  | source
(spi/read-opt channel &opt kw ...)

Set SPI Read transfer options. Takes zero, or more keywords:

:size-in-bits      - Transfer size in bits (default is bytes)

:cs                - Chip-select line asserted before beginning transfer
spi/readwrite
cfunction  | source
(spi/readwrite channel size sendbuf recvbuf)

Simultaneously read & write size n-bytes to channel.

Returns bytes transfered. Sets :err to return status.
Note: Uses the write-opt transfer option for both operations.
This is a blocking function.

And this is the block above verbatim; note some sections retain the double-space, others sit flat. (Had to \ escape the inner quotes to get it to render here, but otherwise the same)
  ## spi/read-opt

  **cfunction**  | [source][31]

  \```janet
  (spi/read-opt channel &opt kw ...)
  \```

  Set SPI Read transfer options. Takes zero, or more keywords:

* `:size-in-bits`      - Transfer size in bits (default is bytes)
* `:cs`                - Chip-select line asserted before beginning transfer

  [31]: c/spi.c#L331

  ## spi/readwrite

  **cfunction**  | [source][32]

  \```janet
  (spi/readwrite channel size sendbuf recvbuf)
  \```

  Simultaneously read & write `size` n-bytes to `channel`.

Returns bytes transfered. Sets `:err` to return status.

Note: Uses the `write-opt` transfer option for both operations.

This is a **blocking function**.

  [32]: c/spi.c#L505

@strangepete Thanks for that. Sorry, I gather this can be ‘fixed’ by just changing the template but I’m curious to see the input, too. Is that publicly available somewhere? If not, would it be possible to put one of the files that you’re using to generate the doc in a Gist?

Of course, I completely forgot! I'll try to put the whole thing up later today, but this is one of the submodules
The whole spi.c:  https://gist.github.com/strangepete/762fcf853669ebc00e99009ff697fca1
I'll include just the fragments of what generates the above docstring below (internally out of order from output)
JANET_FN(cfun_spi_set_read_options,
    "(spi/read-opt channel &opt kw ...)",
    "Set SPI Read transfer options. Takes zero, or more keywords:\n\n"
    "* `:size-in-bits`      - Transfer size in bits (default is bytes)\n"
    "* `:cs`                - Chip-select line asserted before beginning transfer\n\n") {
    janet_arity(argc, 1, 3);

    channel_t *c = (channel_t *)janet_getabstract(argv, 0, &channel_type);
    c->read_options = spi_transfer_option_keywords(argc, argv);
    return set_status_dyn(FT_OK, janet_wrap_nil());
}
-----
JANET_FN(cfun_spi_readwrite,
    "(spi/readwrite channel size sendbuf recvbuf)",
    "Simultaneously read & write `size` n-bytes to `channel`.\n\n"
    "Returns bytes transfered. Sets `:err` to return status.\n\n"
    "Note: Uses the `write-opt` transfer option for both operations.\n\n"
    "This is a **blocking function**.") {
    janet_fixarity(argc, 4);
[...]

@strangepete Thanks for that. I’d like to see if I can land a fix in Janet that would support dedenting on Windows platforms so will see how that goes first. Apologies for the delay!

No need for apology! That'd be fantastic, so I'll close that pull again :D

All set with #1534, just installed a fresh copy and output looks perfect. Thank you for the effort, much appreciated!

No problems! Glad it got there in the end. Look forward to a new official release coming soon :)


move function documentation to header

I passionately dislike libraries, which put the user API documentation into the c files. It clearly belongs into the header files, where the user (or users IDE) can find it, even when precompiled lib + headers are used. In addition there might be several implementations for different architectures in the future, each having its c file. Still you want a common interface definition for them maintained in a single set of headers.
This is of course a kind suggestion, but with a strong opinion ;-)

Pal uses doxygen for documenting code. It's output is considered as a standalone product (pdf, html, etc.) you can view without source code, hence your previous argument is, kinda, invalid :stuck_out_tongue:
I agree with your point that having docs in header files makes it easier with maintenance. Particularly in case of multiple definitions for different architectures. However, even at this point of development, pal is a huge library, so moving docs to headers would be a tedious job :tired_face:, which I, presonally, would not take up.


Issues with mouse tracking when dismissing a popup by clicking on the top bar

I noticed a weird behavior of the mouse after a certain sequence happens.
You will see in the movie that buttons hovering is broken, but there's more issues when opening menus for instance.
Here is how to reproduce it.

Unfold a widget like a combo box, a toolmenu or a QMenu
Click in the title bar (but not in a section that has been processed by FramelessWindowsManager::setHitTestVisible()
Now the mouse tracking is broken

It can be fixed by reopening the widget and clicking somewhere else, or my dragging the window or even by clicking outside the window.

It seems something is happening in FramelessHelperWin::nativeEventFilter()

I can reproduce this BUG in Win10. I guess that any widget (such as QMenu, QComboBox) that pops up through exec() will cause this BUG. It seems like nativeEventFilter() can't handle quitting of event loops of these widgets.

@altairwei where would you advise me to look? Which message type might be at fault?

where would you advise me to look? Which message type might be at fault?

Sorry, I don't have any ideas at the moment.

Returnin false in case WM_NCHITTEST: removes the bug

@altairwei It seems I can't reproduce the bug if the window is maximized, do you confirm?

Could this help?
https://github.com/lambdaxymox/wine/commit/cfcc280905b7804efde8f42bcd6bddbe5ebd8cad

This patch #93 worked, thanks a lot!

@altairwei I'm sorry I already asked this question and can't find the answer: can you remind me what's the purpose of your 2.0 branch? Are there bugfixes that I should cherry pick in my form of 1.0. Should I work with branch 2.0? Thanks a lot.

@JulienMaille The purpose of 2.0 has been explained in https://github.com/wangwenx190/framelesshelper/issues/86#issuecomment-934139020 . The implementation of 2.0 on Windows is similar to that of 1.0, but 2.0 has some minor refactoring to fit the new API design. Cherry-picking directly would have been a bit tricky, as I had removed or moved some of the code elsewhere.
The initial work on 2.0 is now complete and I have applied it to the WizNotePlus project. If you are interested, try out the altairwei/2.0 branch and decide if you want to work on it.

Thanks and sorry for asking twice, totally forgot about your other reply

Thanks and sorry for asking twice, totally forgot about your other reply. Is this commit in 1.0? 1f1de5d

I'm not sure. This BUG is caused by Utilities::getSystemMetric. @wangwenx190 forgot to fix function calls when change it's function signature.
By the way, altairwei/2.0 branch is serveral commits behind 1.x

@JulienMaille  Nothing need to be cherry-picked back to the 1.x branch. For Windows, 2.0 is only a redesign and reorganization of the project, the core code doesn't change. The main goal of 2.0 is to provide complete support for Linux and macOS without relying on Qt 5.15's QWindow API, which is not related to Windows.

This BUG is caused by Utilities::getSystemMetric. @wangwenx190 forgot to fix function calls when change it's function signature.

@altairwei  这个问题现在还存在吗

这个问题现在还存在吗

That BUG #77 has been fixed in altairwei/2.0, but I'm not sure if it has also been fixed in 1.x

Thanks and sorry for asking twice, totally forgot about your other reply. Is this commit in 1.0? 1f1de5d

@JulienMaille The issue you mentioned is now fixed in the 1.x branch. Sorry for the inconvenience.


PHPCBF bug: foreach no brace with ${$var['key']}

Original code:
foreach ( $item['postmeta'] as $meta )
	${$meta['key']} = $meta['value'];

After phpcbf with PHP 7.1.7, this results in:
foreach ( $item['postmeta'] as $meta ) {
	$meta['key']} = $meta['value'];

Instead of:
foreach ( $item['postmeta'] as $meta ) {
	${$meta['key']} = $meta['value'];
}

Report:
  938 | ERROR   | [x] Opening brace should be on the same line as the declaration (WordPress.WhiteSpace.ControlStructureSpacing.OpenBraceNotSameLine)
  938 | ERROR   | [x] Expected 1 space(s) after closing parenthesis; found "\n            $" (Squiz.ControlStructures.ControlSignature.SpaceAfterCloseParenthesis)
  939 | ERROR   | [x] Newline required after opening brace (Squiz.ControlStructures.ControlSignature.NewlineAfterOpenBrace)
  939 | ERROR   | [x] Array keys must NOT be surrounded by spaces if they only contain a string or an integer. (WordPress.Arrays.ArrayKeySpacingRestrictions.SpacesAroundArrayKeys)
  939 | ERROR   | [x] Array keys must NOT be surrounded by spaces if they only contain a string or an integer. (WordPress.Arrays.ArrayKeySpacingRestrictions.SpacesAroundArrayKeys)

This looks to me likes a known upstream issue with the Generic.ControlStructures.InlineControlStructure sniff which is getting confused over the curly braces of the variable variable.
See: https://github.com/squizlabs/PHP_CodeSniffer/issues/1882

FYI: a fix has just been committed upstream which should take care of this. The fix will be included in the 3.3.0 release. If you would happen to have the chance to test it, that'd be great!

@GaryJones Can this issue be closed ? The issue was with PHPCS itself & has been fixed upstream.


update docker usage documentation

I sligthly modified the readme:

I added a section to place the docker file in your desired location where you want to start of with the docker image creation. I could not find this in the readme.
I had to run the commands for docker build and docker run with sudo rights. This may also require an update.
Also, I simply installed docker and docker.io from repo without adding the latest version as per docker documentation which is referred to in the readme. Maybe we could say something like use these packages and either follow the docker instructions, if this does not work for your distro or just give it a try with the few packages.
Next step can be to refer the documentation in the meta-elisa readme readme.md as a good way to start the build. Especially, the script takes away some work for you.
Change from koi to needlefish once merged

If you follow the steps at
https://docs.docker.com/engine/install/linux-postinstall/ then you will not
need sudo.
I have updated dockerfile for needlefish, will mail you so that you can
test it and place it in the appropriate place,
--
Regards
Sudip

Add the sudo handling as additional information to the docker docu to close this item.
Location of modification is here:  https://github.com/elisa-tech/wg-automotive/tree/master/Docker_container

non-root handling mentioned at: https://github.com/elisa-tech/meta-elisa/issues/31

As per the previous comment the non-root handling is mentioned in the Troubleshooting section run docker as non-root user
This should be added to the documentation for the docker container at the end of the docker installation section https://github.com/elisa-tech/wg-automotive/tree/master/Docker_container#install-docker
It should come after Install docker from your distro or you can follow instructions at https://docs.docker.com/engine/install/ and be mentioned as: "As a post installation step following the instructions at Manage Docker as a non-root user enables building and running Docker as non-root user."

@LeandraVoges improved the documentation and the commit is merged into master. This issue is closed.


[BUG] Switch Profiles results in Git error message

Syncify Version
I am using Syncify v3.9.1
Describe the bug
When I attempt to switch Syncify profiles, I am presented with a Git-related error message (see screenshot). I'm pretty sure I've diagnosed the issue, so see below.
To Reproduce
Steps to reproduce the behavior:

Started with "Syncer" setting set to "Repo", successfully uploaded my settings to the default profile.
Created a second profile on a different Git branch.
Open Command Palette and execute the "Syncify: Switch Profile" (resulted in a warning message, but I assumed that I did something incorrectly, and so unfortunately didn't capture the message contents.)
Restarted VS Code (optional? I did it in response to the above warning.)
Confirmed in my Syncify settings that the current profile switched to the new one, despite the above warning.
Modified VS Code settings.
Open Command Palette and execute the "Syncify: Upload" command (success)
Open Command Palette and execute the "Syncify: Switch Profile" command (results in the error in the screenshot below)

Expected behavior
Switch Profile succeeds.
Screenshots

Additional context
(Windows 10, VS Code v1.41.0, Git v2.20.1)
I believe that this is caused by the version of Git that I have installed. Based on a quick Google search, it appears that the "git switch" command was added in v2.23, but my machine is running v2.20.1. At the time of writing, the latest version is v2.24.1.
There appear to be two options to fix: either simply document that the extension requires a certain version of Git, or update the extension to use backward-compatible Git commands.
Unfortunately, I am unable to upgrade Git on this particular machine, but I will test on a different machine and provide an update when I get a chance.

@dxman Can you try reproducing it with this version? VSIX.

Sure thing.
After installing the VSIX, I ran the "Switch Profile" command, the status bar showed the "Downloading..." message, and it appears to have completed with no errors. I immediately opened my settings.json file, but there did not appear to be any changes, so I checked the "Last Modified" timestamp in Windows Explorer. It still showed yesterday's date.
I then restarted VS Code and attempted to switch back to my original profile. At that point, I got the following error message:

Just let me know if there's any other useful info I can provide.

@dxman I just published v3.9.3, which should fix this problem by using checkout instead of switch on old git versions. We can keep this issue open for a bit to make sure that the fix works on your machine.

Apologies for going silent; was away for the holidays. Confirmed that the latest version is working for me.


Rename appveyor.yml to appyor.yml

Nazer

appveyor.yml

Why this change? This will break CI/CD completely.

I will this. thanks


Unable to access standard report date item from report extension data item

Please include the following with each issue:
1. Describe the bug
Unable to access standard report ("Add Contacts") data item "Contact" from report extension data item (Service Item) because text field in Service Item table is called "Contact" as well and only option i have is that field.
2. To Reproduce
Steps to reproduce the behavior:

Create report extension for standard report "Add Contacts"
Add data item to report extension with table "Service Item"
Try to access data item "Contact" in "Service Item" data item

AL code snippet that demonstrates the issue or a link to a code repository the developers can easily pull down to recreate the issue locally.

Note: Because the developers need to copy and paste the code snippet, including a code snippet as a media file (i.e. .gif) is not sufficient.
3. Expected behavior
Standard data item "Contact" can be accessed from other data items on report extension
4. Actual behavior

5. Versions:

AL Language: 2204
Visual Studio Code: 1.86.0
Business Central: 22.04
List of Visual Studio Code extensions that you have installed: AL CodeActions, AL Language extension for Microsoft Dynamics 365 Business Central, AL Variable Helper

Final Checklist
Please remember to do the following:

[ ] Search the issue repository to ensure you are reporting a new issue

[ ] Reproduce the issue after disabling all extensions except the AL Language extension

[ ] Simplify your code around the issue to better isolate the problem

Just add CurrReport. in front of Contact.

@dannoe Thanks a lot for contributing here and helping people! I'll close this bug.


[Bug]:  Using the dall-e 3 notebook example code, the request data is too large.

Describe the bug
I tried to integrate the dalle-3 example code to my groupchat bot,  (https://github.com/microsoft/autogen/blob/main/notebook/agentchat_dalle_and_gpt4v.ipynb),
but after the dalle agent replied with img data in base64 format, the next request encounter an error sad request data too large.
I think it's because the autogen push the reply data of base64 img to message list, in the next request, there will be a large request body.
How to exclude some message or truncate it in the group chat?

Steps to reproduce
No response
Expected Behavior
No response
Screenshots and logs
No response
Additional Information
No response

@kevin666aa

Hello @deerleo, when you are creating dalle agent, are you using your own customized agents? A quick solution is to process the image in the customized registered reply function. You will take out the image and return a message like: "Placeholder for Image".

@BeibinLi

@deerleo  Thanks for the feedback. The recent truncate update might interfere with how we handle LMM.
I will take a deeper look into this.

@deerleo  Can you check: https://github.com/microsoft/autogen/pull/1124
Also, can you make the chat_manager a MultimodalAgent instead of a conversable agent?

@BeibinLi I don't think there is a MultimodalGroupChat?
You can add groupchat manager capabilities to any agent via registration, but I suspect the problem lies with the GroupChat class that handles the orchestration.

@afourney Got it. You are correct. I just added an issue (https://github.com/microsoft/autogen/issues/1142) regarding MultimodalGroupChat. I will create a MultimodalGroupChat under the multimodal features.


Publish the utility package

The utility and utility.os_ext packages provide generic functionality that can be useful for test implementors. I suggest fine tuning their documentation and publishing those as well.

Also I suggest renaming os_ext to osext. This should not affect the users. It's purely internal. It will affect though some of CSCS tests, but this is just CSCS-specific.


[Feature] Remove Allies Info from Allies page

User Story
As an ally
I want to have my information protected
So I can have peace of mind
Acceptance Criteria

[ ] The  ally cards are no longer present
[ ] The information is still stored an able to be used for those seeking allies

Is your feature request related to a problem? Please describe.
This came up due to the possibility of these names getting scarped with their info and used for maybe not good purposes. We want to Allies to be able to connect to the community, but in a different way.
Describe the solution you'd like
Allies moves to the resources page and when a user needs an ally in specific industry, we can provide a resource taking on new people (still working through what v1 for this will be)
Other
New designs will be needed to show what this will look like

We have established that our conversion of Allies to Resources is likely not going to make it into V1. Our focus will remain on Business architecture for now.


eChecklists not exporting properly when multiple ST&Es exist.

After importing and categorizing the same data into two different ST&Es, one of the ST&Es no longer exports eChecklists properly - it creates the file but it has no eChecklist tabs, only Cover Sheet and Instructions.
I was able to replicate this issue using the sample eChecklist in the shared folder named "Win_10_-_x64-100hosts-modified-eChecklist-29Nov18.xlsx" (I trimmed off hosts after JOAB-67, not sure if it would work with less).
Steps to reproduce:

Create two Systems, two Sites, and two ST&Es.  The data I used was Test System, Test Site, and ST&E 1 (the pre-loaded test data) and Test System 2, Test Site 2, and ST&E 2.
Import trimmed checklist into first ST&E. Auto-categorize. Verify export works correctly.
Import trimmed checklist into second ST&E. Auto-categorize.  Click export button.  File exports but checklist tabs are not present.

Digging a little it looks like get_cat_findings only has hosts in it with tgt_id < 66 (i.e., none of the hosts from the second import).  This probably explains the symptom... are you matching findings on import based only on hostname and not also ST&E ID?

Digging a little it looks like get_cat_findings only has hosts in it with tgt_id < 66 (i.e., none of the hosts from the second import). This probably explains the symptom... are you matching findings on import based only on hostname and not also ST&E ID?

Targets have to be linked directly to an ST&E ID, so they are unique as necessary.
Were you able to figure anything else out about this?

No, just like I said, you can replicate it by importing the same unclass echecklist with ~67 hosts into two separate ST&Es.

From: Ryan P<EMAIL_ADDRESS>Sent: Tuesday, February 12, 2019 6:16 PM
To: cyberperspectives/sagacity
Cc: mshuter; Author
Subject: Re: [cyberperspectives/sagacity] eChecklists not exporting properly when multiple ST&Es exist. (#96)
Digging a little it looks like get_cat_findings only has hosts in it with tgt_id < 66 (i.e., none of the hosts from the second import). This probably explains the symptom... are you matching findings on import based only on hostname and not also ST&E ID?
Targets have to be linked directly to an ST&E ID, so they are unique as necessary.
Were you able to figure anything else out about this?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com/cyberperspectives/sagacity/issues/96#issuecomment-463013592, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AsBz0x_kJaDHNl3G-1T9kOEHgDilsU0dks5vM2d7gaJpZM4afGgW.


Cannot close inventory using inventory keybind

In Creative Mode, there is a separate keybind to open the player inventory instead of the Creative inventory. However, you cannot press this separate keybind to close the player inventory. Ideally, you could use both E or I (or the rebound equivalnet) to close any inventory screen.

Fixed in version 1.4, thanks for reporting.


Implement Notification Support

Summary
This pull request adds notification support to the Aurora Editor, enhancing user engagement and interaction with the editor. It includes the implementation of notification components, API integration, and user interface enhancements to provide a seamless user experience.
Changes Made

Notification Components

Added notification components including Notification, NotificationList, and NotificationItem.
These components allow for the creation, display, and management of notifications within the Aurora Editor.

API Integration

Integrated with the Aurora Editor's backend API to fetch and display relevant notifications for the user.
Added API endpoints for creating and managing user-specific notifications.

User Interface Enhancements

Enhanced the user interface to accommodate the notification feature.
Added a notification icon/button to the header for easy access.
Implemented a notification dropdown to display and interact with notifications.

User Engagement

Notifications are triggered for various user events such as extension updates, account updates and project issues.
Users can mark notifications as read or dismiss them as needed.

TODO

[ ] Allow power users to select how long a notification shows for.
[ ] Allow users to choose what types of notifications are displayed.
[ ] Make the notification API more robust and future-proof.
[ ] Allow notification icons to open the notification navigator view.
[ ] Optional: Explore the possibility of allowing the notification system to be active or disabled based on the user's system focus.

Motivation
The primary goal of this PR is to improve user engagement and interaction within the Aurora Editor. Notifications play a crucial role in keeping users informed about relevant activities and updates, ultimately enhancing their experience.
Testing

Extensive testing has been performed to ensure the reliability and stability of the notification feature.
Unit tests cover the notification components, API endpoints, and notification-related functionality.
Integration tests were conducted to verify that notifications are triggered correctly in response to various user actions.

Screenshots

Checklist

[x] Code has been tested and verified.
[x] Documentation has been updated to reflect the changes.
[x] All tests pass successfully.
[x] Code follows the project's coding guidelines and style.
[x] The branch is up-to-date with the latest changes from the main branch.
[ ] Reviewed by at least one other contributor.

Related Issues
None
Reviewers

@0xWDG
@Maartz

Additional Notes
None

@aurora-editor-bot please approve.


Reapply [CodeCompletion] Migrate some tests to batch completion test #4

Reapply b810bb7bbdfb38fc5e8942d3562edc318e06cc3d but without test/IDE/complete_where_clause.swift which caused non-deterministic failures.

@swift-ci Please smoke test

@swift-ci Please smoke test Linux


[VideoTranslation][ClientSampleCode][Python] Initial check in client sample code for python.

Purpose
Initial check in client sample code for python
Pull Request Type
What kind of change does this Pull Request introduce?
[ ] Bugfix
[x ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

/azp run


S3 Interface: Multipart ETags(?) causing checksum mismatch

Some of the files on the Cloudian (and probably other S3 backends) seem to have some kind of multipart identifier attached to the ETag - which is weird by itself, because I thought I didn't upload them as multipart...
Anyway, the suffix confuses the checksum calculation.
Example:
cf = CloudFiles("matrix://fafbv14-em/aligned/v1/16_16_40")
x = cf["129.shard"]

has a checksum of cd8d2616dfa6cc80a06a846d3b3f6f30-14. ~Ignoring the -14 part results matches the expected checksum.~ Was wrong about that.

Yeah, somehow those files must have been uploaded as multipart... not sure why.
ETag calculation is a bit annoying: the number after the dash is the number of multiparts for that object. With that and the size of the full object, one can guess the size of a each multipart and calculate the MD5 for each part + the final combined MD5.
E.g.: https://teppen.io/2018/10/23/aws_s3_verify_etags/

That's odd, if these were created using CloudFiles. I wasn't aware it could even do that. Did you use s3cli or gsutil? Those can sometimes automatically switch to multipart.

I am fairly sure that I did those with CloudVolume - it's the sharded version of Flywire. But I had to rework some files a while ago - maybe I used gsutil there, don't remember.
The real issue is that I can't read them with CloudFiles, though.  For now, I set etag = None in the interfaces.py for those etags that contain the multipart suffix, so that my transfer finishes.

I'm surprised CF can download the multipart files without special logic. I was considering the multi-part logic in #7

In any case, this is a good feature. I'll look into it some more. Thanks for the test case and info.


When using IIS clientCertificateMappingAuthentication IIS properly authorizes user in AD but Request.HttpContext.User does not reflect that

Describe the bug
When using IIS clientCertificateMappingAuthentication IIS properly authorizes user in AD but Request.HttpContext.User does not reflect that
To Reproduce
Using this version of ASP.NET Core '2.2'
Use IIS Hosting. Configure web site in the following manner:
<location path="SiteName">
    <system.webServer>
        <security>
            <authentication>
                <windowsAuthentication enabled="false" />
                <anonymousAuthentication enabled="false" />
                <basicAuthentication enabled="false" />
	    <clientCertificateMappingAuthentication enabled="true" />
            </authentication>
            <access sslFlags="Ssl, SslNegotiateCert, SslRequireCert" />
        </security>
    </system.webServer>
</location>

Configure services in the following way
services.AddAuthentication(IISDefaults.AuthenticationScheme);
services.AddAuthorization();
For http pipeline:
app.UseAuthentication();
Result:
In controller method
Request.HttpContext.User IS NOT populated with windows principal
Request.HttpContext.Connection.ClientCertificate IS populated
IIS log files correctly show domain user, ie first request gets response 403.7, then after supplying certificate from browser IIS logs show correct user in the cs-username column.
Expected behavior
In controller method
Request.HttpContext.User IS populated with windows principal
Request.HttpContext.Connection.ClientCertificate IS populated
IIS log files correctly show domain user, ie first request gets response 403.7, then after supplying certificate from browser IIS logs show correct user in the cs-username column.
Additional context
.NET Core SDK (reflecting any global.json):
Version:   2.2.104
Commit:    73f036d4ac
Runtime Environment:
OS Name:     Windows
OS Version:  10.0.17134
OS Platform: Windows
RID:         win10-x64
Base Path:   C:\Program Files\dotnet\sdk\2.2.104\
Host (useful for support):
Version: 2.2.2
Commit:  a4fd7b2c84
.NET Core SDKs installed:
2.1.504 [C:\Program Files\dotnet\sdk]
2.2.104 [C:\Program Files\dotnet\sdk]
.NET Core runtimes installed:
Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.All 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

Are you using IIS In-process or out-of-process? We'd need to verify the behavior of both.

I am using in process handler.

We don't support the clientCertificateMapping feature right now. We do have native client certificate auth coming in 3.0 (see our preview6 announcement)


Update the guide and API for two-way binding with v-with in 0.11.5

Why was it changed to a unidirectional flow in 0.11 anyway? Why was it changed back?

Thanks a lot!


Update Available signals without restart

Is your feature request related to a problem? Please describe.
The signals list only updates at start up.
Describe the solution you'd like
Create a function to allow for updates to the list without restart.

From @mLayneSpectric:  Files (local and Azure) never resync, causing the server to need a restart


Automatic type creation on module boundaries

Flow can infer a lot of type information automatically, but unfortunately that isn't shared across module boundaries, which I do understand. However, it would be great if there is some way to assign automatically inferred types to a type name, which currently isn't possible AFAIK. For example, consider the following snippet:
function objectFactory() {
    return {
        myProperty : 3
    };
}

console.log(objectFactory().myProperty * 3);

This snippet checks fine. However, consider the following variation on the snippet:
export function objectFactory() {
    return {
        myProperty : 3
    };
}

This now gives the error:
  2: export function objectFactory () {
                                     ^ return. Missing annotation

As far as I know the only way to work around this is by defining the return type of the factory explicitely, i.e.:
type MyType = {
    myProperty : number
};

export function objectFactory() : MyType { /* etc */ }

Certain styles of Javascript programming make use of factory functions like these extensively. In such code bases having to create explicit types for all these functions result in a lot of boilerplate. It would therefore be great if there was some way to automatically assign a name to the inferred type, e.g. something like this:
export function objectFactory() : =MyType { /* etc */ };

This would eliminate all of the boilerplate.
Would such a thing be possible? Or are there practical reasons why this can't be done?

This is fixed, closing


add place from order

Add some form where can add place

define in storage static restaurant name
define in storage static few meal (with proce and desc) which user can seletc
display defined meal in user form

merge to develop


pretraining epochs

Hi, how many epochs have you pretrained for the resnet50 checkpoint you provided? I noticed that the ema of batch norm parameters was counted to 625568, but I don't know the batch size you used in pretraining.

Hi, they've been trained for 1000 ImageNet epochs

Got it! Thank you very much.


move unwrapping and wrapping of paths to the gateway

We've moved the wrapping and unwrapping of reference paths to the storage gateway so that the storage provider doesn't have to know its mount path.
To test this with ocis you need these changes: https://github.com/owncloud/ocis/pull/2460

fixes https://github.com/cs3org/reva/issues/578

started an acceptance test in https://github.com/cs3org/reva/pull/2219

Tests from @butonic in #2219 are merged. Can we continue on this one?

merged in edge as part of https://github.com/cs3org/reva/pull/2234


Add missing 'sysinfoapi' feature for winapi

stdlib doesn't compile on Windows MSVC x64 otherwise.

I believe we're in the process of transitioning away from the winapi crate towards the windows crate.
cc @youknowone

@youknowone I've updated the email address in the commits to the one on Github.

Thank you!


Add GitHub workflow to publish docs on GitHub

This will update https://ripple.github.io/rippled automatically any time we push to the develop branch. We can always push to the gh-pages branch manually to override it.
You can see an example of the current output at https://thejohnfreeman.github.io/rippled.
I experimented with Read The Docs before this. RTD supports browsing multiple versions of documentation via its Sphinx theme, but the HTML output of Doxygen is completely unaware of this theme. Even if we were ok with supporting just a single version, getting Sphinx to use the raw HTML output of Doxygen requires a custom builder, but RTD only lets us choose one of the three built-in builders (html, dirhtml, and singlehtml).

Codecov Report

Merging #3302 into develop will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3302   +/-   ##
========================================
  Coverage    70.61%   70.62%
========================================
  Files          676      676
  Lines        54330    54340   +10
========================================
+ Hits         38365    38375   +10
  Misses       15965    15965

Impacted Files
Coverage Δ

src/ripple/app/consensus/RCLCxPeerPos.cpp
29.03% <0.00%> (-0.97%)
:arrow_down:

src/ripple/shamap/impl/SHAMapTreeNode.cpp
78.84% <0.00%> (-0.70%)
:arrow_down:

src/ripple/app/consensus/RCLConsensus.cpp
59.17% <0.00%> (-0.15%)
:arrow_down:

src/ripple/app/ledger/Ledger.cpp
79.26% <0.00%> (-0.13%)
:arrow_down:

src/ripple/ledger/impl/View.cpp
91.49% <0.00%> (-0.02%)
:arrow_down:

src/ripple/protocol/Feature.h
100.00% <0.00%> (ø)

src/ripple/protocol/HashPrefix.h
100.00% <0.00%> (ø)

src/ripple/protocol/Serializer.h
85.71% <0.00%> (ø)

src/ripple/nodestore/impl/codec.h
35.84% <0.00%> (ø)

src/ripple/app/misc/NetworkOPs.cpp
64.00% <0.00%> (ø)

... and 9 more

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 f76a5a3...f0df325. Read the comment docs.


Fix issue 15864 - chmgen triggers exception in std.regex

This was a release blocker I believe  so please review/merge ASAP.

Seems like I was wrong after all and stable is not affected


chore: add @kocoa/adapter-mocha project

Codecov Report

:exclamation: No coverage uploaded for pull request base (next@0939722). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             next      #55   +/-   ##
=======================================
  Coverage        ?   94.98%
=======================================
  Files           ?       20
  Lines           ?      837
  Branches        ?       50
=======================================
  Hits            ?      795
  Misses          ?       38
  Partials        ?        4

Flag
Coverage Δ

Packages
97.27% <0.00%> (?)

Samples
78.84% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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 0939722...a256184. Read the comment docs.


Bash completions don't work

Problem
The Bash completions don't work because they are not be sourced correctly.
Steps
Rustup expects Bash completions in ~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/etc/bash_completion.d/cargo:
$ rustup completions bash cargo
if command -v rustc >/dev/null 2>&1; then
        source "$(rustc --print sysroot)"/etc/bash_completion.d/cargo
fi

Unfortunately, the completions are not there:
$ ls -d "$(rustc --print sysroot)"/etc/bash_completion.d/cargo
ls: cannot access '/home/stefan/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/etc/bash_completion.d/cargo': No such file or directory

But, the completions are here:
$ file "$(rustc --print sysroot)"/src/etc/bash_completion.d/cargo
/home/stefan/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/src/etc/bash_completion.d/cargo: ASCII text

Note the /src/etc instead of just /etc.
Possible Solution(s)
The Bash completions should be placed in /etc/bash_completion.d/cargo instead of /src/etc/bash_completion.d/cargo.
Notes
No response
Version
$ cargo --version
cargo 1.71.0 (cfd3bbd8f 2023-06-08)

Thanks for the report, and sorry for the inconvenience. This is a regression and planned to get a stable backport soon. See https://github.com/rust-lang/rustup/issues/3407. Will keep this issue open till a new point release comes out (presumbly 1.71.1).
@rustbog label -S-triage +S-blocked-external +A-completions

Fixed in 1.71.1 with https://github.com/rust-lang/rust/pull/113579. Thanks you for reporting this!


QSFS Docs

Related Issue

As explained here: https://github.com/threefoldtech/home/issues/1539

TODO:

make a new section in the manual based on this repo: https://github.com/threefoldtech/quantum-storage?tab=readme-ov-file
update the javascript and terraform qsfs docs:

For terraform:

The docs.
The example

For TS client:

The docs.
The example.

WIP: https://github.com/threefoldtech/info_grid/tree/development_quantum_storage

boostrap, done
script, done
ubuntu, wip

Status: Almost done. Scott is doing an amazing job with the bash work to go with ubuntu and tfcmd. Will be able to make a PR soon.

Status: Almost done still. We just need to document for the mycelium case and we can release the guide after the 3.14 update is live (the guide uses new stuff from 3.14, as Mycelium network in VMs).

We can close when #588 is merged.

@scottyeager can you give an update on the situation so everyone is up to speed with the status? Thanks!

Status

WIP
Things need to be fixed at the grid/zos/mycelium level until we can finish the QSFS docs.

Update

Scott and I will try to get something working for 3.15 release, so within 5 days or so
Then we can check how to improve the rest on further issues

Update

Scott (and I to some extent) will work on the QSFS pulumi version and aim to get a working version before the 12th of November

Update

Scott is finalizing the docs for QSFS

Update

Scott is still working on QSFS with the help of Iwan. Basically, lots of issues need to be fixed to have a 100% coherent and functioning QSFS guide and setup

@scottyeager feel free to add info here or state the progress/status. Thanks!

Update

Scott is still working on this
It's not easy as Mycelium has issues in terms of connectivity
Still WIP


Where should I put the config file?

Sorry but after reading the readme I still have no clue how I am supposed to config this plugin. It seems like people are using some lua script to config, but I haven't found anyone mentioning where to put that file.

I installed it with dein, and no commands like LLMToggleAutoSuggest are available after installing.

I just struggled a little bit as well, since my config still uses vimscript. Turns out, there is a way to embed lua in vimscript:
lua << EOF
any lua code here
EOF

Since I use ollama backend, here is what I ended up for a config:
[...more stuff here of course...]
Plug 'huggingface/llm.nvim'
call plug#end()

lua << EOF
require('llm').setup({
  backend="ollama",
  url = "...",

  lsp = {
    cmd_env = { LLM_LOG_LEVEL = "DEBUG" }
  },

  model='deepseek-coder-v2:16b-lite-base-q4_0',
  fim = {
    enabled = true,
    prefix = '<｜fim▁begin｜>',
    suffix = '<｜fim▁hole｜>',
    middle = '<｜fim▁end｜>',
  },
})
EOF

It works OK, the only issue is fim needs to be selected properly for each model. I have also noticed mysterious errors returned by ollama by some prompts, but I'm not sure exactly where they come from.


play-angular-standalone doesn't compile

Hi,
play-angular-standalone example does not compile using jwt-play v3.1.0.

[error]     request.jwtSession.getAs[User]("user") match {```

Unfortunately, I barely have time (and motivation) to maintain the lib itself, I will rely on the community and pull requests for updating those examples. If you would have time to open one, that would be awesome!

Fixed

@pauldijou tanks.