feat: tweak test setup, add some cases

Codecov Report

Merging #5 (18a0df7) into main (a755f72) will increase coverage by 28.15%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             main       #5       +/-   ##
===========================================
+ Coverage   39.08%   67.23%   +28.15%
===========================================
  Files           3        3
  Lines         174      177        +3
===========================================
+ Hits           68      119       +51
+ Misses        106       54       -52
- Partials        0        4        +4

Impacted Files
Coverage Δ

fluentgraphql.go
54.33% <100.00%> (+39.81%)
:arrow_up:

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 a755f72...18a0df7. Read the comment docs.


Typo -- Press Create, not OK

The article says, "Name the application MyGuestApp and press OK." In Visual Studio 2019 the button is labeled Create.

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

ID: 878614ff-821c-323b-105a-c40471d6a420
Version Independent ID: fa9df7f4-0d7d-7a74-e38d-a2302a0070f0
Content: Quickly deploy an existing app to a cluster - Azure Service Fabric
Content Source: articles/service-fabric/quickstart-guest-app.md
Service: service-fabric
GitHub Login: @athinanthny
Microsoft Alias: atsenthi

Thanks for the feedback! We are currently investigating and will update you shortly.

Thanks for bringing this to our attention. I have created a PR with the update, changes will be live shortly.


[Feature] Assign batch splits for ContinuousFileSplitEnumerator

Search before asking

[X] I searched in the issues and found nothing similar.

Motivation
#687 introduces an option scan.split-enumerator.batch-size for assigning splits at one time to avoid exceed akka.framesize limit in StaticFileStoreSplitEnumerator. ContinuousFileSplitEnumerator should support this mechanism too.
Solution
No response
Anything else?
No response
Are you willing to submit a PR?

[ ] I'm willing to submit a PR!

Hi @FangYongs  Assign this to me .

This is solved, please close~


verify: remove extra calls to rekor for verify and verify-blob

Signed-off-by: Asra Ali<EMAIL_ADDRESS>
Summary

Removes extra calls to Rekor during verification:

When verifying a blob, we retrieve the entry UUIDs (by an artifact hash search), and then call verifyRekorEntry, which then takes the (cert, pubkey, artifact) and makes a call to rekor to get the entry. Instead, we pass the entry since we already have it.

Distangles VerifyTLogEntry from inside of FindTlogEntry. Users can call this directly. This means that we don't need to FindTlogEntry to verify it if we already have it, which saves the call from (1).

In both verify and verify-blob, we call verifyRekorEntry, which has ANOTHER call to GetTlogEntry after FindTlogEntry to get the actual entry, we already got it! Refactored FindTlogEntry to give you back the... entry.

Move the UUID verification into functions that are asking for an entry by UUID (e.g. when you get or find an entry)

Ticket Link

Fixes
Release Note

cc @rohankh532 this makes VerifyTLogEntry public, see the verify-blob commands for usage

Codecov Report

Merging #1694 (4ffc10f) into main (b099136) will decrease coverage by 0.08%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1694      +/-   ##
==========================================
- Coverage   29.43%   29.34%   -0.09%
==========================================
  Files         141      141
  Lines        8413     8428      +15
==========================================
- Hits         2476     2473       -3
- Misses       5668     5685      +17
- Partials      269      270       +1

Impacted Files
Coverage Δ

cmd/cosign/cli/verify/verify_blob.go
10.49% <0.00%> (-0.14%)
:arrow_down:

pkg/cosign/tlog.go
4.33% <0.00%> (-0.22%)
:arrow_down:

pkg/cosign/verify.go
18.45% <0.00%> (+0.03%)
:arrow_up:

pkg/cosign/tuf/client.go
62.34% <0.00%> (-0.95%)
:arrow_down:

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 b099136...4ffc10f. Read the comment docs.


Implement ranking system and update Home page

Feature Overview:
Added a ranking system to highlight top contributors based on the number of posts.
Created a new Ranking page accessible from the Home page.
Introduced a podium-style layout to showcase the top 3 contributors.
Changes Made:
Home Page (Home.js):
Added a "View Rankings" button that navigates to the new ranking page.
Integrated navigation link into the existing page structure.
Ranking Page (Ranking.js):
Implemented a podium layout for the top 3 contributors with color-coded tiers (Gold, Silver, Bronze).
Listed all contributors in descending order of posts in a clean, readable format.
Styled the page using consistent design patterns from the project (e.g., using Poppins font and box shadows).

after merging this login page has disappeared


Additional TI sources

Please provide information about how we can use TI feeds from other sources

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

ID: 1dcaaa43-666b-38eb-a95a-7fe2cd2b613b
Version Independent ID: 49c83dd4-9de6-1827-cefa-6c6838ce280b
Content: Azure Firewall threat intelligence configuration
Content Source: articles/firewall-manager/threat-intelligence-settings.md
Service: firewall-manager
GitHub Login: @vhorne
Microsoft Alias: victorh

Thank you for your dedication to our documentation.
Unfortunately, we have been unable to review this issue in a timely manner. We sincerely apologize for the delayed response. We are closing this issue. If you feel that the problem persists, please respond to this issue with additional information.
Please continue to provide feedback about the documentation. We appreciate your contributions to our community.
#please-close


"Find All References" produces wrong line number for jumps

For every entry in the list the line number is one line above the right line number. E.g. the first should be line 3 and not 2:

Should be a zero index issue somewhere :wink:

https://github.com/onivim/oni/blob/105c707d156438deb85e95724a0df3365845f72b/vim/core/oni-plugin-typescript/src/FindAllReferences.ts#L22-L40
I'm not fully sure what that code does, but it looks to be the culprit.
Add 1 to line and col, do the look up and then remove 1 from both.
Getting rid of all the minus 1s seems to fix it for me though:
Left is with the fix and the cursor is on the very start of Configuration, right is without and its one line above and the col is off.

If I'm not missing something, I can submit the PR for it, but I assume I probably am?

Thanks for the detailed investigation, @CrossR!
This is confusing because there are 3 entities involved, and each has a different indexing behavior (in other words, uses either zero-based line/columns, or one-based line columns):

Language Server Protocol - uses zero-based lines/columns
TypeScript Standalone Server - uses one-based lines/columns
Neovim (quickfix) - uses one-based lines/columns

So in the code above - what's happening is the request comes in via the language server protocol, so we have to convert it to a one-based index for the typescript server, and then the typescript server sends us back one-based indices, so then we have to re-map it back to the protocol that the language client expects.
However, once we get those zero-based indices, we hand them off to Neovim in the quickfix window, without adjusting back to one-based indices (which Neovim expects). So it looks like the root issue is here:
https://github.com/onivim/oni/blob/6501a2b50d690ffe31e66aca71a5972c79e3e4eb/browser/src/Services/Language/FindAllReferences.ts#L49
When we're converting from types.Location to the quick-fix item format, we should be converting from zero-based to one-based indices.
And it looks like it is impacting other languages too (like if I 'Find all references' in a .less file):

Just added a PR for this now finally.
As a side note, is there a second bug in that screenshot?
Oni is highlighting and shows a search for container, but the LSP is doing a search for overlay-container as far as I can tell. Its especially noticeable if you do it on background-image a few lines down whilst highlighting background. Every instance of the word background highlighted on the line, but the LSP is only searching for background-image. The LSP functionality makes sense, I think the highlighting and search term just need fixing to reflect it too?

Forgot to stick an update in this issue!
Bug should be fixed now, and the latest release should have this update.
Have you noticed if its working @hoschi ?

@CrossR still sitting on an old version because of some bugs/missing features, but I should be able to update with the solarized theme now. I put this on my list to check it, when I updated. You can close it in the meantime, if you want.


FileTarget: performance improvement: 10-70% faster, less garbage collecting (3-4 times less) by reusing buffers

FileTarget and AsyncTargetWrapper with optimized memory allocations for layout rendering, encoding and streaming. Optimization is disabled by default for all targets with the setting OptimizeBufferReuse (Default = false). It has been explicit enabled for pure NLog-FileTarget and pure NLog-AsyncTargetWrapper.
Introduced a new property AsyncTargetWrapper.FullBatchSizeWriteLimit (Default = 5). It gives better performance to have many small writes. But have increased AsyncTargetWrapper.BatchSize to 200.
Refactoring of #1663

This change is

Current coverage is 82% (diff: 83%)

Merging #1799 into master will increase coverage by <1%

@@             master      #1799   diff @@
==========================================
  Files           274        282     +8
  Lines         18256      18833   +577
  Methods        2834       2901    +67
  Messages          0          0
  Branches       2091       2176    +85
==========================================
+ Hits          14851      15368   +517
- Misses         2923       2974    +51
- Partials        482        491     +9

Powered by Codecov. Last update 9ca0515...a13f388

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
5.450
1.834.774
0
16
33
7.875
87,215

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
12.019
831.993
0
15
30
14.421
93,309

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfSyncLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
6.675
1.497.936
0
0
158
6.625
35,516

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfSyncLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
13.432
744.466
0
0
146
13.093
35,516

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
8
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
16.811
594.842
0
43
97
21.265
112,004

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
16.182
617.962
0
46
97
21.062
106,359

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
15.881
629.681
0
34
70
31.359
156,430

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
16
2
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
30.680
325.942
0
96
218
64.359
108,250

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
3.000.000
16
4
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
18.414
162.914
0
55
140
67.062
109,598

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
3.000.000
16
8
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
62.459
48.031
0
56
114
235.484
152,988

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SyncLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
19.918
502.052
0
0
458
19.906
35,793

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SyncLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
26.542
376.751
0
0
642
26.453
35,949

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
8
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
15.295
653.775
0
21
42
17.640
93,004

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
15.720
636.097
0
23
46
18.359
90,414

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
22.184
450.761
0
21
43
24.234
96,363

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
16
2
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
32.017
312.327
0
39
78
42.734
98,621

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
3.000.000
16
4
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
30.461
98.486
0
22
45
60.390
96,551

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
3.000.000
16
8
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
136.431
21.989
0
45
91
414.921
94,750

this one supersedes https://github.com/NLog/NLog/pull/1663?
I will review/test it soon! I will include this in the next release after 4.4 as this is delaying 4.4.
could you do a performance test to release/4.4 ?

** Release 4.4 **
Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
7.125
1.403.333
0
46
92
10.140
110,004

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
16.190
617.658
0
120
240
20.421
131,137

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfSyncLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
7.065
1.415.394
0
0
461
7.250
35,352

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

PerfSyncLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
14.460
691.518
0
0
1173
14.578
35,945

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
8
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
21.770
459.343
0
100
245
34.000
117,648

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
22.678
440.938
0
101
252
36.750
115,367

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
19.815
504.662
1
149
297
40.656
153,922

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
10.000.000
16
2
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
33.538
298.164
0
209
565
72.968
115,012

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
3.000.000
16
4
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
27.254
110.073
0
73
158
98.406
145,395

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

Logger
3.000.000
16
8
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
86.818
34.554
0
210
593
331.015
152,996

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SyncLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
19.112
523.213
0
0
838
19.203
35,816

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SyncLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
27.558
362.867
0
1
1571
26.828
36,117

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
8
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
19.334
517.201
0
75
150
22.000
127,414

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
16
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
19.819
504.564
0
78
156
22.937
128,336

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
128
1
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
31.401
318.455
0
162
325
35.859
136,402

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
10.000.000
16
2
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
40.773
245.254
0
150
301
53.015
130,344

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
3.000.000
16
4
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
31.356
95.675
0
93
187
55.531
124,395

Executing warmup run...
Executing performance test...

Logger Name
Messages
Size
Threads
Loggers

SimpleLogger
3.000.000
16
8
1

Test Name
Time (ms)
Msgs/sec
GC2
GC1
GC0
CPU (ms)
Mem (MB)

My Test
159.223
18.841
0
172
344
549.781
130,320

Reviewed 72 of 73 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 12 unresolved discussions.

src/NLog/Internal/AppendBuilderCreator.cs, line 42 at r2 (raw file):

    /// Allocates new builder and appends to the provided target builder on dispose
    /// </summary>
    internal struct AppendBuilderCreator : IDisposable

why a struct?

src/NLog/Internal/FilePathLayout.cs, line 172 at r2 (raw file):

            {
                _layout.RenderAppendBuilder(logEvent, reusableBuilder);
                if (_cachedPrevRawFileName != null && _cachedPrevRawFileName.Length == reusableBuilder.Length)

what are we trying to do here? (please add some comments)

src/NLog/Internal/ResuableBuilderCreator.cs, line 42 at r2 (raw file):

    /// Controls a single allocated StringBuilder for reuse (only one active user)
    /// </summary>
    internal class ResuableBuilderCreator

Resuable=> Reusable (multiple classes)

src/NLog/Internal/FileAppenders/WindowsMultiProcessFileAppender.cs, line 49 at r2 (raw file):

    /// </summary>
    [SecuritySafeCritical]
    internal class WindowsMultiProcessFileAppender : BaseFileAppender

This is already in release/4.4? Is this due a squash commit?

src/NLog/LayoutRenderers/Wrappers/Rot13LayoutRendererWrapper.cs, line 79 at r2 (raw file):

        /// </summary>
        /// <param name="encodedValue">The string to be encoded/decoded.</param>
        public static void DecodeRot13(System.Text.StringBuilder encodedValue)

why public?

src/NLog/Layouts/Layout.cs, line 74 at r2 (raw file):

        }

        private const int MaxInitialRenderBufferLength = 16384;

why this value? Please add XML doc for this choice

src/NLog/Targets/FileTarget.cs, line 2398 at r2 (raw file):

            if (layout == null)
            {
                return default(ArraySegment<byte>);

Why is this better then null?

src/NLog/Targets/Target.cs, line 63 at r2 (raw file):

        /// <summary>
        /// Gets or sets the name of the target.

update docs :)
Please explain WHEN or WHY we need it (in general) - or add pro and cons. Thanks

src/NLog/Targets/Target.cs, line 632 at r2 (raw file):

                try
                {
                    if (!OptimizeBufferUsage && logEvents.Count == logEvents.Array.Length)

why the check logEvents.Count == logEvents.Array.Length?

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, line 172 at r2 (raw file):

        /// Gets or sets the limit of full <see cref="BatchSize"/>s to write before yielding
        /// </summary>
        /// <docgen category='Buffering Options' order='100' />

please explain why/when we need this or pros/cons in xmldoc . thx!

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, line 394 at r2 (raw file):

(continuation != null

replace with variable? (3 times used)x

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, line 448 at r2 (raw file):

        }

        private int WriteLogEventBatch(ArraySegment<AsyncLogEventInfo> logEvents, AsyncContinuation continuation)

please document return value

Comments from Reviewable

great test results! Is this by running https://github.com/NLog/NLog-Performance-tests/blob/master/run-nlogperformance-tests.bat ?

src/NLog/Internal/AppendBuilderCreator.cs, line 42 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
why a struct?

Because then it will only allocate memory on the stack

Comments from Reviewable

src/NLog/Internal/FilePathLayout.cs, line 172 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
what are we trying to do here? (please add some comments)

I'm avoiding string allocation for every LogEvent-write

Comments from Reviewable

src/NLog/Internal/ResuableBuilderCreator.cs, line 42 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Resuable=> Reusable (multiple classes)

Good catch, will fix

Comments from Reviewable

src/NLog/Internal/FileAppenders/WindowsMultiProcessFileAppender.cs, line 49 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
This is already in release/4.4? Is this due a squash commit?

No the PR is pointer to master, instead of 4.4

Comments from Reviewable

Great test results by the way :)

src/NLog/LayoutRenderers/Wrappers/Rot13LayoutRendererWrapper.cs, line 79 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
why public?

No reason. I can make it internal

Comments from Reviewable

src/NLog/Layouts/Layout.cs, line 74 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
why this value? Please add XML doc for this choice

Just copy/paste code.

Comments from Reviewable

src/NLog/Targets/FileTarget.cs, line 2398 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Why is this better then null?

Because ArraySegment is a struct and has to be created.

Comments from Reviewable

src/NLog/Targets/Target.cs, line 632 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
why the check logEvents.Count == logEvents.Array.Length?

Well someone calls with a logEvents-ArraySegment that only specifies a sub-part of the array, then it has to use the new style

Comments from Reviewable

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, line 172 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
please explain why/when we need this or pros/cons in xmldoc . thx!

Done.

Comments from Reviewable

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, line 394 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…

(continuation != null

replace with variable? (3 times used)x

Done.

Comments from Reviewable

src/NLog/Targets/Wrappers/AsyncTargetWrapper.cs, line 448 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
please document return value

Done.

Comments from Reviewable

src/NLog/Targets/Target.cs, line 63 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
update docs :)
Please explain WHEN or WHY we need it (in general) - or add pro and cons. Thanks

Done.

Comments from Reviewable

@304NotModified Is this by running NLog-Performance-Tests?

 No severe performance impact detected.

great work!

Will try to search for some time so it could be included in 4.4.0.
Do we need to write docs on this?

Do we need to write docs for this?

Not besides maybe finding a better name than OptimizeBufferUsage. If we are planning it to be the default behavior. That custom target wrappers that depend on logevent-array-clones being made automatically, has to do it manually.

Changed the name of the setting to RestrictedBufferReuse, so it gives a feeling of legacy options.

Reviewed 33 of 41 files at r3, 7 of 7 files at r4.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

src/NLog/LoggerImpl.cs, line 87 at r4 (raw file):

                if (ex != null)
                {
                    if (LogManager.ThrowExceptions && Thread.CurrentThread.ManagedThreadId == originalThreadId)

this is wrong I guess. You could use multiple log configurations and thus having other LogFactory objects

src/NLog/Internal/AppendBuilderCreator.cs, line 42 at r2 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Because then it will only allocate memory on the stack

but we aren't passing as parameter?

Comments from Reviewable

Sorry this is taking so long as have again conflicts :(

src/NLog/Internal/AppendBuilderCreator.cs, line 42 at r2 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
but we aren't passing as parameter?

Not sure I understand, the AppendBuilderCreator creates scope where it makes a StringBuilder available. And if the input StringBuilder is clean as a whistle then it skips the allocation.

Comments from Reviewable

Sorry this is taking so long and have again conflicts :(

My own fault, that I'm making new PR that conflict with this one :). Merged again

src/NLog/LoggerImpl.cs, line 87 at r4 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
this is wrong I guess. You could use multiple log configurations and thus having other LogFactory objects

Thought one could only have one LogManager with one LogFactory. Have reverted the change.

Comments from Reviewable

Reviewed 28 of 29 files at r5.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

src/NLog/LoggerImpl.cs, line 87 at r4 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Thought one could only have one LogManager with one LogFactory. Have reverted the change.

thx

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):

        /// <docgen category='Performance Tuning Options' order='10' />
        public bool RestrictedBufferReuse { get { return _restrictedBufferReuse; } set { _restrictedBufferReuse = value; } }
        private bool _restrictedBufferReuse = true;

is there a reason why this isn't an auto property?

src/NLog/Targets/Wrappers/AsyncRequestQueue-T.cs, line 137 at r5 (raw file):

            lock (this)
            {
                if (count == -1 || this.logEventInfoQueue.Count < count)

is this not needed anymore?

Comments from Reviewable

I think we're almost there!
But unfortunately I'm missing a high-level overview.

Does this change introduce new config settings?
If there are issues, could we still disable this feature (by config)
Is this fully backwardscompatible for custom targets/layout renderers etc? (I think so)

Thanks! Will try to merge soon, but tomorrow my holiday start, not sure if I have (good) internet there ;)

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
is there a reason why this isn't an auto property?

Wanted the default value = true, without creating a constructor. Default constructor is better?

Comments from Reviewable

src/NLog/Targets/Wrappers/AsyncRequestQueue-T.cs, line 137 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
is this not needed anymore?

Yes changed the magic-value from -1 to int.MaxValue

Comments from Reviewable

Does this change introduce new config settings?

And the new setting AsyncTargetWrapper.FullBatchSizeWriteLimit

Comments from Reviewable

Hi @snakefoot
Happy new year!
I like to finish/merge this one first before all the other PRs. Unfortunately it's hard to get a good feeling about the changes in the (github) review, but I think I'm almost there.
Added some notes, will do a full review again to be sure.
I was looking from the config viewpoint. I have the following questions:

We only could deactivate it per target? Not globally? It the latter not wise in case of problems?
I'm not sure if the term RestrictedBufferReuse is the best. Was more thinking of MinimalMemoryUsage or just ReuseBuffers. What do you think?

Thanks in advance!
I'm still kinda busy, will think we could merge this one next week :)

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

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Wanted the default value = true, without creating a constructor. Default constructor is better?

Please do, a protected contructor I think? It's more consistent with the rest!

src/NLog/Targets/Wrappers/AsyncRequestQueue-T.cs, line 137 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Yes changed the magic-value from -1 to int.MaxValue

this is a breaking change? Or it's fixed somewhere else?

Comments from Reviewable

Hi @snakefoot
Happy new year!
I like to finish/merge this one first before all the other PRs. Unfortunately it's hard to get a good feeling about the changes in the (github) review, but I think I'm almost there.
Added some notes, will do a full review again to be sure.
I was looking from the config viewpoint. I have the following questions:

We only could deactivate it per target? Not globally? It the latter not wise in case of problems?
I'm not sure if the term RestrictedBufferReuse is the best. Was more thinking of MinimalMemoryUsage or just ReuseBuffers. What do you think?

Thanks in advance!
I'm still kinda busy, will think we could merge this one next week :)

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

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Wanted the default value = true, without creating a constructor. Default constructor is better?

Please do, a protected contructor I think? It's more consistent with the rest!

src/NLog/Targets/Wrappers/AsyncRequestQueue-T.cs, line 137 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Yes changed the magic-value from -1 to int.MaxValue

this is a breaking change? Or it's fixed somewhere else?

Comments from Reviewable

Review status: all files reviewed at latest revision, 13 unresolved discussions.

src/NLog/Internal/AppendBuilderCreator.cs, line 47 at r5 (raw file):

        /// Access the new builder allocated
        /// </summary>
        public readonly StringBuilder Builder;

would it harm to go here to a public getter?
Not mandatory, but prefer the rule "don't expose fields"

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 52 at r5 (raw file):

        }

        public LockList Allocate()

docs please

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 57 at r5 (raw file):

        }

        public readonly LockList None;

docs please. Why we would use none?N

src/NLog/Internal/ReusableBufferCreator.cs, line 45 at r5 (raw file):

        private char[] _buffer;

        public readonly LockBuffer None;

consider a getter

src/NLog/Internal/ReusableBufferCreator.cs, line 52 at r5 (raw file):

        }

        public LockBuffer Allocate()

would an internal interface not be nice? For the docs /understanding at least?

src/NLog/Internal/ReusableBuilderCreator.cs, line 48 at r5 (raw file):

        public readonly LockBuilder None;

        public LockBuilder Allocate()

same comments as before

src/NLog/LayoutRenderers/Wrappers/WrapperLayoutRendererBuilderBase.cs, line 95 at r5 (raw file):

        protected sealed override string Transform(string text)
        {
            throw new System.NotImplementedException("Use TransformFormattedMesssage");

consider NotSupportedException

src/NLog/LayoutRenderers/Wrappers/WrapperLayoutRendererBuilderBase.cs, line 105 at r5 (raw file):

        protected sealed override string RenderInner(LogEventInfo logEvent)
        {
            throw new System.NotImplementedException("Use RenderFormattedMessage");

Consider NotSupportedException

src/NLog/Targets/DebugTarget.cs, line 103 at r5 (raw file):

        {
            this.Counter++;
            if (RestrictedBufferReuse)

I would expect NOT RestrictedBufferReuse here. It's also in other targets. What am I missing? I was thinking:

When RestrictedBufferReuse = true, use the resuableBuilders / allocatie

src/NLog/Targets/FileTarget.cs, line 1102 at r5 (raw file):

                    int bucketCount = bucket.Value.Count;

                    using (var targetBuilder = !RestrictedBufferReuse ? ReusableLayoutBuilder.Allocate() : ReusableLayoutBuilder.None)

consider an c# extension (if we could make a interface)

tests/NLog.UnitTests/Layouts/AllEventPropertiesTests.cs, line 52 at r5 (raw file):

            var ev = BuildLogEventWithProperties();

            renderer.RenderAppendBuilder(ev, sb);

Couldn't we just calling .Render? (same for other changes)

Comments from Reviewable

Review status: all files reviewed at latest revision, 13 unresolved discussions.

src/NLog/Internal/AppendBuilderCreator.cs, line 47 at r5 (raw file):

        /// Access the new builder allocated
        /// </summary>
        public readonly StringBuilder Builder;

would it harm to go here to a public getter?
Not mandatory, but prefer the rule "don't expose fields"

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 52 at r5 (raw file):

        }

        public LockList Allocate()

docs please

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 57 at r5 (raw file):

        }

        public readonly LockList None;

docs please. Why we would use none?N

src/NLog/Internal/ReusableBufferCreator.cs, line 45 at r5 (raw file):

        private char[] _buffer;

        public readonly LockBuffer None;

consider a getter

src/NLog/Internal/ReusableBufferCreator.cs, line 52 at r5 (raw file):

        }

        public LockBuffer Allocate()

would an internal interface not be nice? For the docs /understanding at least?

src/NLog/Internal/ReusableBuilderCreator.cs, line 48 at r5 (raw file):

        public readonly LockBuilder None;

        public LockBuilder Allocate()

same comments as before

src/NLog/LayoutRenderers/Wrappers/WrapperLayoutRendererBuilderBase.cs, line 95 at r5 (raw file):

        protected sealed override string Transform(string text)
        {
            throw new System.NotImplementedException("Use TransformFormattedMesssage");

consider NotSupportedException

src/NLog/LayoutRenderers/Wrappers/WrapperLayoutRendererBuilderBase.cs, line 105 at r5 (raw file):

        protected sealed override string RenderInner(LogEventInfo logEvent)
        {
            throw new System.NotImplementedException("Use RenderFormattedMessage");

Consider NotSupportedException

src/NLog/Targets/DebugTarget.cs, line 103 at r5 (raw file):

        {
            this.Counter++;
            if (RestrictedBufferReuse)

I would expect NOT RestrictedBufferReuse here. It's also in other targets. What am I missing? I was thinking:

When RestrictedBufferReuse = true, use the resuableBuilders / allocatie

src/NLog/Targets/FileTarget.cs, line 1102 at r5 (raw file):

                    int bucketCount = bucket.Value.Count;

                    using (var targetBuilder = !RestrictedBufferReuse ? ReusableLayoutBuilder.Allocate() : ReusableLayoutBuilder.None)

consider an c# extension (if we could make a interface)

tests/NLog.UnitTests/Layouts/AllEventPropertiesTests.cs, line 52 at r5 (raw file):

            var ev = BuildLogEventWithProperties();

            renderer.RenderAppendBuilder(ev, sb);

Couldn't we just calling .Render? (same for other changes)

Comments from Reviewable

TODO for this PR:

[ ] reviewed all the code
[ ] squash commits
[ ] last review after squash
[ ] double check breaking changes (there is a tool for it?)
[ ] finish all xmldocs. Others should understand it
[ ] check/discuss naming

TODO for this PR:

[ ] reviewed all the code
[ ] squash commits
[ ] last review after squash
[ ] double check breaking changes (there is a tool for it?)
[ ] finish all xmldocs. Others should understand it
[ ] check/discuss naming

We only could deactivate it per target? Not globally? It the latter not wise in case of problems?

Think I will change the logic so it will be activated by default, when running the official NLog-targets and NLog-target-wrappers.

I'm not sure if the term RestrictedBufferReuse is the best. Was more thinking of MinimalMemoryUsage or just ReuseBuffers. What do you think?

Will change the name to allow AllowBufferReuse instead.

Comments from Reviewable

We only could deactivate it per target? Not globally? It the latter not wise in case of problems?

Think I will change the logic so it will be activated by default, when running the official NLog-targets and NLog-target-wrappers.

I'm not sure if the term RestrictedBufferReuse is the best. Was more thinking of MinimalMemoryUsage or just ReuseBuffers. What do you think?

Will change the name to allow AllowBufferReuse instead.

Comments from Reviewable

src/NLog/Internal/AppendBuilderCreator.cs, line 47 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
would it harm to go here to a public getter?
Not mandatory, but prefer the rule "don't expose fields"

Well everything is Internal so nothing is accessible, and it is a struct.

Comments from Reviewable

src/NLog/Internal/AppendBuilderCreator.cs, line 47 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
would it harm to go here to a public getter?
Not mandatory, but prefer the rule "don't expose fields"

Well everything is Internal so nothing is accessible, and it is a struct.

Comments from Reviewable

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 52 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
docs please

Added

Comments from Reviewable

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 52 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
docs please

Added

Comments from Reviewable

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 57 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
docs please. Why we would use none?N

Docs added

Comments from Reviewable

src/NLog/Internal/ReusableAsyncLogEventList.cs, line 57 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
docs please. Why we would use none?N

Docs added

Comments from Reviewable

src/NLog/Internal/ReusableBufferCreator.cs, line 45 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
consider a getter

It is internal and has the same meaning as TimeSpan.Zero

Comments from Reviewable

src/NLog/Internal/ReusableBufferCreator.cs, line 45 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
consider a getter

It is internal and has the same meaning as TimeSpan.Zero

Comments from Reviewable

src/NLog/Internal/ReusableBufferCreator.cs, line 52 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
would an internal interface not be nice? For the docs /understanding at least?

No opinion about this. Think the comments and the usage is very clear to me

Comments from Reviewable

src/NLog/Internal/ReusableBufferCreator.cs, line 52 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
would an internal interface not be nice? For the docs /understanding at least?

No opinion about this. Think the comments and the usage is very clear to me

Comments from Reviewable

src/NLog/Internal/ReusableBuilderCreator.cs, line 48 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
same comments as before

Added

Comments from Reviewable

src/NLog/Internal/ReusableBuilderCreator.cs, line 48 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
same comments as before

Added

Comments from Reviewable

src/NLog/LayoutRenderers/Wrappers/WrapperLayoutRendererBuilderBase.cs, line 105 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Consider NotSupportedException

Done

Comments from Reviewable

src/NLog/LayoutRenderers/Wrappers/WrapperLayoutRendererBuilderBase.cs, line 105 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Consider NotSupportedException

Done

Comments from Reviewable

src/NLog/Targets/DebugTarget.cs, line 103 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
I would expect NOT RestrictedBufferReuse here. It's also in other targets. What am I missing? I was thinking:

When RestrictedBufferReuse = true, use the resuableBuilders / allocatie

Renamed to AllowBufferReuse

Comments from Reviewable

src/NLog/Targets/DebugTarget.cs, line 103 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
I would expect NOT RestrictedBufferReuse here. It's also in other targets. What am I missing? I was thinking:

When RestrictedBufferReuse = true, use the resuableBuilders / allocatie

Renamed to AllowBufferReuse

Comments from Reviewable

src/NLog/Targets/FileTarget.cs, line 1102 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
consider an c# extension (if we could make a interface)

There can be other conditions that prevents us from activating the reusable-object, see how AsyncTargetWrapper handles flush. And how FileTarget skips using the reusable-objects when a large flush arrives (logEvents.Count <= 1000)

Comments from Reviewable

src/NLog/Targets/FileTarget.cs, line 1102 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
consider an c# extension (if we could make a interface)

There can be other conditions that prevents us from activating the reusable-object, see how AsyncTargetWrapper handles flush. And how FileTarget skips using the reusable-objects when a large flush arrives (logEvents.Count <= 1000)

Comments from Reviewable

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Please do, a protected contructor I think? It's more consistent with the rest!

Renamed to AllowBufferReuse with default value false, except for the pure FileTarget.

Comments from Reviewable

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Please do, a protected contructor I think? It's more consistent with the rest!

Renamed to AllowBufferReuse with default value false, except for the pure FileTarget.

Comments from Reviewable

tests/NLog.UnitTests/Layouts/AllEventPropertiesTests.cs, line 52 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Couldn't we just calling .Render? (same for other changes)

I will revert the tests. But the plan is to make Render an obsolete method in NLog ver. 5

Comments from Reviewable

tests/NLog.UnitTests/Layouts/AllEventPropertiesTests.cs, line 52 at r5 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
Couldn't we just calling .Render? (same for other changes)

I will revert the tests. But the plan is to make Render an obsolete method in NLog ver. 5

Comments from Reviewable

Have squashed the commits.
Have also verified the assembly using BreakCop. It detects a breaking change in a protected method on a sealed class. Since no one from outside can call this method by normal means then it is not a breaking change.

Have introduced the protected method Target.RenderLogEvent.
Just for fun then I have tried using it on the NLog-targets. I can remove it again from the targets, and just use it for the DebugTarget if it should be saved for NLog ver. 5

Review status: 45 of 67 files reviewed at latest revision, 13 unresolved discussions, some commit checks failed.

src/NLog/Targets/DebugTarget.cs, line 103 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Renamed to AllowBufferReuse

Changed my mine to OptimizeBufferReuse :)

src/NLog/Targets/Target.cs, line 68 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
Renamed to AllowBufferReuse with default value false, except for the pure FileTarget.

Changed my mine to OptimizeBufferReuse :)

Comments from Reviewable

Reviewed 3 of 41 files at r3, 18 of 21 files at r6, 4 of 4 files at r7.
Review status: all files reviewed at latest revision, 3 unresolved discussions.

src/NLog/LogEventInfo.cs, line 427 at r7 (raw file):

            if (this.layoutCache == null)
            {
                Interlocked.CompareExchange(ref this.layoutCache, new Dictionary<Layout, string>(), null);

nice improvement

src/NLog/Internal/ReusableBufferCreator.cs, line 52 at r5 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
No opinion about this. Think the comments and the usage is very clear to me

OK, maybe add it in the future.

src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs, line 124 at r7 (raw file):
this is a bit problematic with Semver :(

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. It MUST be incremented if any public API functionality is marked as deprecated. It MAY be incremented if substantial new functionality or improvements are introduced within the private code. It MAY include patch level changes. Patch version MUST be reset to 0 when minor version is incremented.

it should be then 4.5. What's your opinion?

Comments from Reviewable

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

src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs, line 124 at r7 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
this is a bit problematic with Semver :(

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. It MUST be incremented if any public API functionality is marked as deprecated. It MAY be incremented if substantial new functionality or improvements are introduced within the private code. It MAY include patch level changes. Patch version MUST be reset to 0 when minor version is incremented.

it should be then 4.5. What's your opinion?

I can remove the obsolete warning, as I just used it because the signature of array[] and IList is very close. I guess NLog users that have created their own custom targets, that overrides Write(Array[]) will get a compile warning, that they should change to Write(IList) instead.

Comments from Reviewable

src/NLog/LogEventInfo.cs, line 427 at r7 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
nice improvement

Thx

Comments from Reviewable

Review status: all files reviewed at latest revision, 1 unresolved discussion.

src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs, line 124 at r7 (raw file):
Previously, snakefoot (Rolf Kristensen) wrote…
I can remove the obsolete warning, as I just used it because the signature of array[] and IList is very close. I guess NLog users that have created their own custom targets, that overrides Write(Array[]) will get a compile warning, that they should change to Write(IList) instead.

doubting. Are you on gitter (https://gitter.im) sometimes?

Comments from Reviewable

src/NLog/Targets/Wrappers/PostFilteringTargetWrapper.cs, line 124 at r7 (raw file):
Previously, 304NotModified (Julian Verdurmen) wrote…
doubting. Are you on gitter (https://gitter.im) sometimes?

Have now removed the obsolete-attribute. And replaced it with a strict xml-comment!

Comments from Reviewable

Reviewed 5 of 8 files at r8, 21 of 21 files at r9.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

Unit test error?

All builds were failing last night, seems things are working again.

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

Comments from Reviewable

merged! finally!

Yeah! I have updated the documentation for the AsyncWrapper:
https://github.com/NLog/NLog/wiki/AsyncWrapper-target

Should I just add OptimizeBufferReuse to FileTarget and AsyncWrapper, since they are the only two targets that enables it by default?

Take fine for me
Thanks in advance!

PS: I always use "Introduced in" instead of "Available". Not sure if if consistency in the docs is that important.

I always use "Introduced in" instead of "Available"

Have updated the docs:
https://github.com/NLog/NLog/wiki/AsyncWrapper-target
https://github.com/NLog/NLog/wiki/File-target


Migration document from aicsimageio

Feature Description
Create a migration document that shows a recipe for switching over from aicsimageio for people who are ready to do so.
Use Case
We wish to encourage users to switch to the new system. Such a document would also allow us to easily see where are the difficult parts of this migration.
Solution
1. Install separate reader packages.
2. Update import statements and replace AICSImage with BioImage.
3. Profit!

A snippet somewhere in this explaining why this change happened might be useful too


Add CV online as hard

Information: https://cvonline.me/en/support/accounts/removing-account

Thanks for your contribution!


Alignment is not correct

Path: Details of Approved Designations by Enam Committee
Issue: The Alignment is not correct.

it is made like other ui

ok


Gas estimation failed

When I try to transfer the tokens to my wallet is says this

How do I fix this?
I am on pragma solidity >=0.5.17;

Hi, can you tell us more ?  The version of Besu you are using, on which network (mainnet, goerli, etc) ? The contract you call if possible as well as the method called etc. This will help us understand your issue

Closing this as the issue was opened 5 months ago and no additional information was shared.


Update README.md

updated the link to medium blog block-element-modifying-your-javascript-components

Thanks for your contribution. However, this has already been fixed on the next branch in #4303.


Query parser fails to parse a range query string when there are escaped brackets inside the range

Description
Assume there's a query parser created, e.g.:
Analyzer analyzer = new ClassicAnalyzer();
QueryParser queryParser = new QueryParser( "field", analyzer );

trying to parse simple ranges like:
// simple range query with no escapes works fine:
Query query = queryParser.parse( "[ 1 TO 10 ]" );

works as expected and a TermRangeQuery is created with no exceptions.
But if the range in the string contains some escaped brackets -- it leads to a parsing exception.
Let's assume one would want to extend the query parser to work with date-time fields and would want to parse something like:
// another range query but now it has some escaping between the range brackets:
query = queryParser.parse( "[ 2024\\-01\\-01T01\\:01\\:01\\+01\\:00\\[Europe\\/Warsaw\\] TO 2025\\-01\\-01T01\\:01\\:01\\+01\\:00\\[Europe\\/Warsaw\\] ]" );

where all special characters are escaped, leads to:
Caused by: org.apache.lucene.queryparser.classic.ParseException: Encountered " "]" "] "" at line 1, column 50.
Was expecting:
    "TO" ...

	at org.apache.lucene.queryparser.classic.QueryParser.generateParseException(QueryParser.java:1004)
	at org.apache.lucene.queryparser.classic.QueryParser.jj_consume_token(QueryParser.java:867)
	at org.apache.lucene.queryparser.classic.QueryParser.Term(QueryParser.java:532)
	at org.apache.lucene.queryparser.classic.QueryParser.Clause(QueryParser.java:366)
	at org.apache.lucene.queryparser.classic.QueryParser.Query(QueryParser.java:251)
	at org.apache.lucene.queryparser.classic.QueryParser.TopLevelQuery(QueryParser.java:223)
	at org.apache.lucene.queryparser.classic.QueryParserBase.parse(QueryParserBase.java:137)
	... 4 more

Note, the idea to do range queries for dates  is to have something along the lines:
QueryParser queryParser = new QueryParser( "field", analyzer ) {
	@Override
	protected Query newRangeQuery(String field, String part1, String part2, boolean startInclusive, boolean endInclusive) {
		var p1 = parseValue(part1);
		var p2 = parseValue(part2);

		return createRangeQueryForDates( field, p1, p2, startInclusive, endInclusive );
	}
};

but because of the parsing error described above, execution never reaches this point.
Version and environment details
Java version: 17.0.9, vendor: Amazon.com Inc.
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.7.10-200.fc39.x86_64", arch: "amd64", family: "unix"
Lucene 9.10.0

You can get around this by placing each range term in quotes:
query = queryParser.parse( "[ \"2024\\-01\\-01T01\\:01\\:01\\+01\\:00\\[Europe\\/Warsaw\\]\" TO \"2025\\-01\\-01T01\\:01\\:01\\+01\\:00\\[Europe\\/Warsaw\\]\" ]" );

In fact, then you don't need to escape anything other than the quotes:
query = queryParser.parse( "[ \"2024-01-01T01:01:01+01:00[Europe/Warsaw]\" TO \"2025-01-01T01:01:01+01:00[Europe/Warsaw]\" ]" );

Both will be parsed to [2024-01-01t01:01:01+01:00[europe/warsaw] TO 2025-01-01t01:01:01+01:00[europe/warsaw]].
(I've added some tests showing this: https://github.com/apache/lucene/pull/13323)

Thanks for looking at this and for the suggestion! I've also tested it out and can confirm that it worked.
I'll let you decide how you'd want to proceed with this ticket (looking at the linked PR, you are considering whether an update to the parser should be applied to support more query string variations)


Drop support for AR < 4, add support for AR = 5

Coverage increased (+0.05%) to 94.273% when pulling ff4a790d0630fcfb75d9cb529a1427a686e408bb on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.05%) to 94.273% when pulling 310409c72febbc7b36ebd8da809f2d31cb291312 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling d46e276ffb943930c061c5b7d63c3fc1b49422ee on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling af967297fd417d098cc948b3c24f0aa79b6d3a63 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling af967297fd417d098cc948b3c24f0aa79b6d3a63 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling af967297fd417d098cc948b3c24f0aa79b6d3a63 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling af967297fd417d098cc948b3c24f0aa79b6d3a63 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling af967297fd417d098cc948b3c24f0aa79b6d3a63 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling af967297fd417d098cc948b3c24f0aa79b6d3a63 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage decreased (-2.3%) to 91.905% when pulling 93bd5914bda95c69d45c1ababfd6658a18a420d7 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+2.7%) to 96.875% when pulling 32f483e9988309b235ad5aad8a2006101ddfec8c on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.

Coverage increased (+0.5%) to 94.762% when pulling 64ed0509622e81aa14ee5391df33d54d92671e03 on chore/drop-activerecord3-support into 2c0621570fd31e08600cc40ff088a224d7971cdb on master.


request (to discuss): link to workflow template from a workflow.

When I view a workflow directly - I sometimes don't know what workflow template it's for and I have to go searching to find it. A link to the workflow template would be handy.

Yes, this would be very useful.


Update to Fullcalendar 3.0.0

Addresses #69
You're not doing anything tricky in the js file right? Just a straight copy and paste? That's what I've done.

Fixed that. The github editor adds an empty last line.

@gr8bit 😄

Absolutely perfect timing folks, I was just now looking for implementing a thing with fullcalendar that's available for 17 days now with 3.0 😁

You guys rock.

Thanks :) Yes looking forward to the new features. The list feature looks great!

Just wondering if you could tag and release a new gem version?


Modularise menu

A continuation of #157. Also adds:

A top-level menu component, to handle the main menu
Use of option generators moved out of main and into separate modules for each menu section
A more generic 'option' class for use with the option generator

Still TODO: all of the logic for changing the data source is still in main - I think this should be pulled out into its own module - if we're in agreement, another issue can be created for this.

I like this.


[UI] Add hover effect to 'About' in Landing Page Footer

Description
I added a hover effect to "About" in Landing Page Footer
Category

[ ] Resource Addition
[ ] Documentation
[ ] Codebase
[x] User Interface
[ ] Feature Addition

Related Issue
Fixes #421
Checklist

[x] I have followed the CONTRIBUTING guide
[x] The name of the resource is spelled correctly (if applicable)
[x] The link to the resource is working (if applicable)
[x] The resource is added in the correct format (if applicable)
[x] I have tested changes on my local computer (if applicable)

Looks good, @Mehdi-Messaadi! 👍


Enable hot module replacement for React via react-hot-loader

What were changed?

Install react-hot-loader and configure it to work with babel
Original index.jsx are renamed to App.jsx and exported as hot module.
New index.jsx are thin shells that calls ReactDOM.render to render <App />

Why?
Hot module replacement for react will enable us to develop the UI faster because when we make changes, the page keeps UI state and only replace the UI code affected by the change instead of reloading the entire page.
For example, you open an SQL editor, look up a few schema, switch tabs. When you modify code for one button and save, everything stays the same. Only the button will be changed.
Reference
Installed based on instruction here
https://github.com/gaearon/react-hot-loader
From the instruction page

Note: You can safely install react-hot-loader as a regular dependency instead of a dev dependency as it automatically ensures it is not executed in production and the footprint is minimal.

@williaster @graceguo-supercat @xtinec @conglei

Codecov Report

Merging #5841 into master will decrease coverage by 0.09%.
The diff coverage is 12.05%.

@@            Coverage Diff            @@
##           master    #5841     +/-   ##
=========================================
- Coverage   63.73%   63.64%   -0.1%
=========================================
  Files         368      374      +6
  Lines       23219    23252     +33
  Branches     2600     2600
=========================================
  Hits        14799    14799
- Misses       8405     8438     +33
  Partials       15       15

Impacted Files
Coverage Δ

...ets/src/visualizations/PairedTTest/PairedTTest.jsx
0% <ø> (ø)
:arrow_up:

superset/assets/src/welcome/index.jsx
0% <ø> (ø)
:arrow_up:

superset/assets/src/dashboard/index.jsx
0% <0%> (ø)
:arrow_up:

superset/assets/src/addSlice/index.jsx
0% <0%> (ø)
:arrow_up:

superset/assets/src/profile/App.jsx
0% <0%> (ø)

superset/assets/src/dashboard/App.jsx
0% <0%> (ø)

superset/assets/src/explore/App.jsx
0% <0%> (ø)

superset/assets/src/addSlice/App.jsx
0% <0%> (ø)

superset/assets/src/SqlLab/App.jsx
0% <0%> (ø)

superset/assets/src/welcome/App.jsx
0% <0%> (-94.45%)
:arrow_down:

... and 10 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 3f8b970...6a1fe8c. Read the comment docs.

Thank you! This one is ready to merge.

LGTM. Well done!


TLS_EMPTY_RENEGOTIATION_INFO_SCSV

Hi,
Got question,
during ssl handshake, client send TLS_EMPTY_RENEGOTIATION_INFO_SCSV (00ff), the leshan-server-demo-2.0.0-M6-jar-with-dependencies.jar can't resolve cipher suite code [ff], and I think this make handshake  is unsuccessful. Logs, tcpdump included.
Am I right?
Regards
Marek
leshanServerLog1.txt

Received 'fatal alert/BAD_CERTIFICATE'

Maybe your device is not happy about leshan's certificate?

TLS_EMPTY_RENEGOTIATION_INFO_SCSV

I will add to ignore that silently. Years ago, we decided, that renegotiation isn't that useful at all for device communication over DTLS.

Validity: [From: Thu Jul 15 15:35:18 CEST 2021,
To: Sat Jun 21 15:35:18 CEST 2121]

and not longer valid, self signed ...
So, I'm not sure, who is operating that host, but the error is similar to zephyr - updatehub cloud. I guess you need to ask there.

Validity: [From: Thu Jul 15 15:35:18 CEST 2021,
To: Sat Jun 21 15:35:18 CEST 2121]

and not longer valid, self signed ...

Why not longer valid? it is valid from 15.07.21 till 21.06.2121, so it is valid now and almost 100 years ahead.

Subject: CN=localhost, O=Internet Widgits Pty Ltd, ST=Some-State, C=AU

So, it's even not leshan's certificate

Why not leshan? it came from java -jar  leshan-server-demo/target/leshan-server-demo-2.0.0-M6-jar-with-dependencies.jar cloned from git clone https://github.com/eclipse/leshan.git and build with mvn clean install
Anyway thanks that TLS_EMPTY_RENEGOTIATION_INFO_SCSV is not issue.
Regards
MM

Internet Widgits Pty Ltd, OK, that's just an example name.
And I guess, this issue is related to Leshan server demo DTLS x.509 mode.
So, you already received the answer, that you need to check your device, why it considers the certificate as BAD.

Thanks,
My client connect without problem in x.509 mode with https://leshan.eclipseprojects.io/#/clients.
I got problem only when I try connect with  leshan server demo running on my pc (<IP_ADDRESS>).
Looking in tcpdump I found that during handshake  with <IP_ADDRESS> in Server Key Exchange it use Named Curve x25519, during hanshake with  leshan.eclipseprojects.io secp256r1 is use so I changed it in Californium3.server.properies (DTLS.CURVES).
x25519 in not included in openssl (1.1.1e in this case)
Now both tcpdumps looks OK,
I changed CN to server IP (<IP_ADDRESS>) but this didn't help.
So it looks like I have to go into openssl and find reason of that issue.
Regards
MM

Thanks,

You're welcome.

So it looks like I have to go into openssl and find reason of that issue.

Do you use libcoap with openssl binding?

My client connect without problem in x.509 mode with https://leshan.eclipseprojects.io/#/clients.
I got problem only when I try connect with leshan server demo running on my pc (<IP_ADDRESS>).

So, which URI do you use, to access your own demo server?
coaps://<IP_ADDRESS>/rd...

coaps://localhost/rd...

coaps://<hostname>/rd...

The first one, with the literal-ip may work, but some tls-x509 implementations expect to only use hostnames in CN, and literal-IP as Alternative Subjects.
One trick around would be, to use the hostname for the certificate and the URI.
Just to say, I would recommend, that you get more common to x509 authentication including certificate-chains and trust-anchors. That would help you a lot to overcome common x509 issues.

Do you use libcoap with openssl binding?
No, I use AVSystem CoAP Library

So, which URI do you use, to access your own demo server?
coaps://<IP_ADDRESS>/rd...

coaps://localhost/rd...

coaps://<hostname>/rd...

This one, coaps://<IP_ADDRESS>:5684
Thanks will try.
Regards
MM

I use AVSystem CoAP Library

OK. Unfortunately, I have no experience with that library and how that configures openssl.

No problems,
I really appreciate your guidance, looks like I will solve it today.
Fingers crossed.
Regards
MM
Hope tomorrow will have nice news.

but some tls-x509 implementations expect to only use hostnames in CN, and literal-IP as Alternative Subjects.
Thanks it works.
Regards
MM

Maybe you tell, what you did, in order to get it working?
Use the hostname in certificate and URI instead of literal IP?
Or how did you set it up to get it working?

URI is like it was coaps://<IP_ADDRESS>:5684
Change is in x509 cert.
CN is server domain name, instead to IP address.
I add subjectAltName = IP:<IP_ADDRESS>
so openssl req -x509 -new -key keys.pem -sha256 -days 36500 -subj '/CN=YOUR_DOMAIN_NAME' -addext "subjectAltName = IP:YOUR_SERVER_IP" -outform DER -out self_signed_cert.der

Thanks!

I added logging support for TLS_EMPTY_RENEGOTIATION_INFO_SCSV in order to prevent users from getting mixed up.
https://github.com/eclipse/californium/pull/1979
I will be released with Californium 3.5.0 and the later, I hope be adapted for one of the leshan 2.0 milestones.

@sbernard31
Maybe worth to be added to Credential files format, that using literal-IPs may require to use also "subjectAltName = IP:YOUR_SERVER_IP".

AFAIK, it's a IETF RFC x509 constraint (one of the countless).
Seems to be the way, openssl verifies x509 certificates.
Anyway, if you think, there is no benefit in explicitly pointing to a proper use of literal-IPs and x509, that's OK for me.

(by the way thanks for support and for proposing this)

AFAIK, it's a IETF RFC x509 constraint (one of the countless).

If one day you fall on this RFC,  do not hesitate to share the link here. (but to not take time to search it on purpose)

Anyway, if you think, there is no benefit in explicitly pointing to a proper use of literal-IPs and x509, that's OK for me.

This is not really that I don't want to point a proper way to do that, this is more that I'm not sure to know exactly what is considering as a good way :grimacing:
About literal-IP, I looked at LWM2M v1.1 specification and it seems the "specified" way to do is to use SNI, see<EMAIL_ADDRESS>Deployments without DNS

In some deployment scenarios, DNS is not used and LwM2M Clients need to use the IP literal, such as coaps://[2001:db8::2:1]/, stored in the "LwM2M Server URI" Resource. To avoid having to use IP addresses also in the Common Name (CN) component of the server certificate or in a field of URI type in the SubjectAltName set this specification uses the Server Name Indication (SNI) Resource.

So maybe it would be better to implement this :point_up:  instead ?

So maybe it would be better to implement this point_up instead ?

The server certificate is verified on the client. And so it depends on the clients implementation. I'm also not sure, if for one or the other (d)tls implementation, this behavior could be configured. But my general experience (even more than a decade ago) was, that even different html browsers did it different. And the most common way was to put it in SAN as IP.
RFC 5280 <IP_ADDRESS> Subject Alternative Name

The subject alternative name extension allows identities to be bound
to the subject of the certificate.  These identities may be included
in addition to or in place of the identity in the subject field of
the certificate.  Defined options include an Internet electronic mail
address, a DNS name, an IP address, and a Uniform Resource Identifier
(URI).  Other options exist, including completely local definitions.
Multiple name forms, and multiple instances of each name form, MAY be
included. Whenever such identities are to be bound into a
certificate, the subject alternative name (or issuer alternative
name) extension MUST be used; however, a DNS name MAY also be
represented in the subject field using the domainComponent attribute
as described in Section <IP_ADDRESS>.

At least, one interpretation of that is, only DNS is allowed to be in the CN, literal-IPs not.

Thx for the inputs.

The server certificate is verified on the client.

Yep I got it but maybe if I document something in Leshan documentation,  this should rather be the LWM2M way :thinking:
And so I also should try to implement it at Leshan Client Side.
I read the RFC 5280 <IP_ADDRESS> Subject Alternative Name twice and I didn't get why this leads to :grimacing:  :

At least, one interpretation of that is, only DNS is allowed to be in the CN, literal-IPs not.

Defined options include an ...  a DNS name, an IP address ...

Whenever such identities are to be bound into a
certificate, the subject alternative name (or issuer alternative
name) extension MUST be used;

=> IP address MUST use SAN

however, a DNS name MAY also be

==> DNS MAy also be in the CN

I get how you get it now.
But not sure to understand the same.
I read this again an the complete sentence is :

however, a DNS name MAY also be
represented in the subject field using the domainComponent attribute
as described in Section <IP_ADDRESS>.

I 'm not sure to even get what is this domainComponent attribute, I don't get this is CN.
Following this that also means that SAN is mandatory for DNS and this is not really what we do for now...
Anyway I keep all of this in mind. Maybe one day I will have a clear understanding of all of this x509 stuff.
This discussion makes me read more about x509 in  LWM2M v1.1 and I feel we are not so good for now.

RFC 7252 - <IP_ADDRESS>

If there is no SubjectAltName in the certificate, then the authority of the
request URI MUST match the Common Name (CN) found in the certificate
using the matching rules defined in [RFC3280] with the exception that
certificates with wildcards are not allowed.

I also don't know, what is common practice and what is well specified.


Update add_args.py

currently, use_auth_token is parsed as as boolean. usually we input the token itself as a string.

I agree that the public version should be the secure one.
huggingface-cli login --token $HUGGINGFACE_TOKEN

Thanks for letting me know that this option exists. I'll move to that.
Regarding the pyproject.toml,
I'll move to ruff for this project.
Would it be sufficient to apply it at the end of the pr(and not each of the commits)?
Anyway, I'll close this PR.


Wiki

Fixed capitalization typos on document save function

Looks good

Thanks for the PR, I was starting a PR for those typos when I saw yours


Wallet - Back button returns to the same page after adding a bank account in Wallet page

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 1.4.54-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team
Action Performed:

Launch New Expensify app.
Go to Profile > Wallet.
Tap Add bank account > Personal bank account.
Add a bank account.
After returning to Wallet page, tap back button on the top left.

Expected Result:
App will return to Profile page.
Actual Result:
App returns to the same Wallet page.
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
https://github.com/Expensify/App/assets/115492554/d92ae0f0-80b4-44f5-8175-abc9be65958e

View all open jobs on GitHub

We think that this bug might be related to the #collect project.

Proposal
Please re-state the problem that we are trying to solve in this issue.
Pressing back button from wallet page returns to the add personal bank account page again.
What is the root cause of that problem?
When we press the continue button after successfully add the bank account, it will call exitFlow which will call goBack with fallback route of wallet page.
https://github.com/Expensify/App/blob/34fa9871c541ccfa03db46bc6e9e428288f718a3/src/pages/AddPersonalBankAccountPage.tsx#L44-L55
Because the add bank account is the only page in the route, it will use the fallback route and push it to the nav stack.
[wallet page, add personal bank account page, wallet page]
So, when we press back, we will arrive at add personal bank account page again.
What changes do you think we should make in order to solve the problem?

Remove the fallback route from goBack.
https://github.com/Expensify/App/blob/34fa9871c541ccfa03db46bc6e9e428288f718a3/src/pages/AddPersonalBankAccountPage.tsx#L54

Proposal
Please re-state the problem that we are trying to solve in this issue.
Wallet - Back button returns to the same page after adding a bank account in Wallet page
What is the root cause of that problem?
The root cause is here :
https://github.com/Expensify/App/blob/34fa9871c541ccfa03db46bc6e9e428288f718a3/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L359
we are just defining Navigation.goBack()
What changes do you think we should make in order to solve the problem?

To move back to Workspace Initial page. We need to modify this route to by adding this :
Navigation.goBack(ROUTES.SETTINGS)

What alternative solutions did you explore? (Optional)
N/A

Updated Proposal

I'll try to test this one tomorrow.

No update yet

On my testing list

I tested this one on Android and see a double click on the back button is required to get to the Profile page.

@eVoloshchak - can you please review if these proposals will resolve this issue? Thanks!

No update yet, this one needs a proposal review.

Looking at this right now

I think we should proceed with @bernhardoj's proposal, as it's the most universal one (we don't rely on setting ROUTES.SETTINGS as a fallback route, which would need to be changed in case we move wallet page outside settings)
🎀👀🎀 C+ reviewed!

PR is ready
cc: @eVoloshchak

Awesome, the PR is moving forward.

I confirmed there is an Upwork job here and will continue with the payment process on Monday.

Payouts due:

[ ] Contributor: $500 @bernhardoj -- paid via Upwork
[ ] Contributor+: $500 @eVoloshchak -- please submit a request

Upwork job is here.

The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/20821
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/20821/files#r1579541672
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: this is a very simple bug, additional discussion isn't needed

Regression Test Proposal

Go to Settings > Wallet.
Tap Add bank account > Personal bank account.
Add a bank account and complete the process until you see the success screen.
Press continue, it will bring you back to wallet page.
After returning to the Wallet page, tap the back button on the top left.
Verify the wallet page is closed

Do we agree 👍 or 👎

$500 approved for @eVoloshchak though re-opening for Al to handle the regression test.

I'll work on the regression test.

Closing - the test request has been created


Make our build pass on travis again

Travis changed some python stuff and now everything is sad.

Fixed!


Improvements

Fixed #92  where not all callbacks were called on the main thread
Unified API to return Result in completions
Code cleanup
Added swiftlint to the project build

Looks OK, but I think that we should move responsibility for dispatching results to (main) queue to the networking library. It makes more sense to me will save us a lot of repetitive code.
I'll create a ticket for that.

Good point, I'll do that right away..


ECS CPU, memory hard limits and host ports are all optional.

http://boto3.readthedocs.io/en/latest/reference/services/ecs.html#ECS.Client.register_task_definition

Thanks!


Invalid$Data<PHONE_NUMBER>004

Hi guys,
I tried to upickle to make my very simple example work:
  case class Post(title: String,
                  content: String,
                  tags: List[String] = List.empty,
                  created: Long = System.currentTimeMillis())
val post = Post("Some title", "some \"content\"", List("algorithms"),<PHONE_NUMBER>004L)
val json = write[List[Post]](List(post))

which causes the following exception:
upickle.Invalid$Data: String (data:<PHONE_NUMBER>004)
The same exception comes when I try to parse son generated externally into that entity.
Json:
[
  {
    "title": "Some fancy title",
    "content": "some very long string with \"escaped\" characters",
    "tags": [
      "algorithms"
    ],
    "created":<PHONE_NUMBER>004
  }
]

Parsing code:
Query.get(
    url = "/posts",
    success = {
      (data: js.Any) =>
        val stringify = JSON.stringify(data)
        console.log(stringify)
        val posts = read[List[Post]](stringify)
        console.log(posts.size)
    }
  )

Also link to the stack overflow question: http://stackoverflow.com/questions/37689453/upickle-invalid-string-value-during-deserialization

Bug bankruptcy


[Feature Request]: 404 Page

Discussed in https://github.com/danielschuster-muc/potter-db/discussions/172
Originally posted by danielschuster-muc October  1, 2022
Project
Website (Potter DB Search)
Describe the feature you want to request
When you get to a page that is not found, you will see a very empty 404 page.
(Optional) Describe a solution for this feature
Edit the 404 page inspired by the image below.
The title of the page should be Have you seen this page?.
In the middle the image with Sirius and below a paragraph with Approach with extreme caution! To not attempt to use magic against this site!
There should also be a button to go back to the homepage.
Make sure to follow the style of other page, use MUI and proper credit the image of Sirius in the Attribution file.

I want to do this

Hey, can I take this issue ?

@PeritusDeveloper on which issue are you currently working? #202 or #204?
Maybe @kirtanchandak can work on one of those, as you are currently assigned to two issues?

@danielschuster-muc are these Hacktoberfest issue ? as you have removed the Hacktoberfest tag from it

@danielschuster-muc are these Hacktoberfest issue ? as you have removed the Hacktoberfest tag from it

All issues / PRs count automatically for hacktoberfest, as this repository is labeled with hacktoberfest.
But please wait for @PeritusDeveloper's reply before working on this feature request.

I am about to finish both
I will submit both by evening tomorrow
If I dont you can assign any one to other person

Sure, no problem. It can also take a bit longer if you need the time. It's just important for me to know whether people are still working on their features, so I can organise it better.

as @PeritusDeveloper is working this issue, can you assign me the other one

I will submit both by evening tomorrow
That sounds for like he is working on both.

@danielschuster-muc you can assign him this issue
it's fine for me
I will do the navbar one

You still working on it @kirtanchandak? Or should I unassign you?


Use nicer punctuation in site-wide banner

Hi Matthew, thanks for your PR.  I've coerced it past CI and I'll get someone to look at this.

I believe this got addressed in the subsequent version of the banner, and it's now been removed anyway. Thanks for your submission, sorry I hadn't replied before now.


--link-mode=symlink and --no-cache are incompatible

We end up persisting to a directory that we then remove. You end up with errors like:
error: Failed to read metadata for: uv==0.2.27
  Caused by: failed to open file `/Users/crmarsh/Library/Application Support/uv/tools/hatch/lib/python3.12/site-packages/uv-0.2.27.dist-info/METADATA`
  Caused by: No such file or directory (os error 2)

We should actually just reject this combination, there's no way to make it work.


Update tiva_timerlow32.c

Missing callback argument field that was preventing successful compilation.

Hi @barbiani thank you for this patch. You need to run the nxstyle and fix all issues reported, probably the original file was already with the issue, but it is important to fix it when submitting a patch.
You can do it this way:

$ cd tools
$ make nxstyle
$ cd ..
$ ./tools/nxstyle arch/arm/src/tiva/common/tiva_timerlow32.c

It will report all the issues that are preventing the CI to pass.

$ make nxstyle

$make -f Makefile.host nxstyle

It complains about a simple 79 columns prototype being line too long...
static uint32_t tiva_usec2ticks(struct tiva_lowerhalf_s *priv, uint32_t
usecs);
I actually inserted the missing field using the github website directly.
On Sat, Sep 5, 2020 at 3:38 PM patacongo<EMAIL_ADDRESS>wrote:

$ make nxstyle
$make -f Makefile.host nxstyle
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/apache/incubator-nuttx/pull/1721#issuecomment-687646590,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABYLTJAM5I2JPC7Y6UZMT4LSEKASPANCNFSM4Q3B7NXA
.

It complains about a simple 79 columns prototype being line too long... static uint32_t tiva_usec2ticks(struct tiva_lowerhalf_s priv, uint32_t usecs); I actually inserted the missing field using the github website directly.
…
On Sat, Sep 5, 2020 at 3:38 PM patacongo @.**> wrote: $ make nxstyle $make -f Makefile.host nxstyle — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1721 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABYLTJAM5I2JPC7Y6UZMT4LSEKASPANCNFSM4Q3B7NXA .

If you prefer you can fix it using the web interface as well, but it is better to do it from your computer because you can revise everything before submitting the PR

I opened PR-1732 to fix the nxstyle issues in this file. After that's merged please rebase this change and let's get it merged.

@hartmannathan PR need rebase not merge.

@hartmannathan PR need rebase not merge.

Agreed. But github is not allowing rebase due to conflicts.
@barbiani, please could you squash/rebase this branch on latest master?

@hartmannathan I fixed the conflict, please review it.


Handle high frequency updates

Hi Arqex, firstly, apologies as this may come across more like a React noob question rather than a technical freezer issue.
One feature of my app is listening for device orientation and a compass is turned accordingly in multiple child components. In turn, the state and UI is updating at a high rate and I can see the affects of jittering and lagging in the affect of the css transform - I believe because I'm force updating the whole app on new orientation.
My question: is there an obvious design pattern I am missing that could be more efficient in doing this? Is it possible to do some checks in my App componentDidMount?
App.js

	componentDidMount() {
		State.on('update', () => this.forceUpdate());
	}
	callbackUpdateOrientation(orientation) {
		State.emit('app:update_orientation', orientation);
	}
	componentWillMount() {
		orientationListener( this.callbackUpdateOrientation.bind(this) );
	}

Reactions.js
	...
	State.get().geo.set({orientation: orientation})

Child.js
	... orientation calculation
	<IconCompass style={{ transform: `rotate(${heading}deg)` }} />;

Many thanks in advance

Hey @digitaloutback ,
Sorry so much for the delay on the reply, maybe you are not even using freezer anymore after seeing how unresponsive I am :/
Your solution is great. It's nice to see how you use the event system, it's exactly the way I have thought of it. So even if the orientation state is really a browser (device) part of data, you update your app state to reflect its current value. And that update refreshes your app.
Freezer's events are used most of the time for tasks related to data fetch/update from the server, but it's cool to see it also in use for changes of state initiated by the device. Another nice use case is to handle websocket events where the server is the one updaing the app state.
Thanks for your interest in the library :)


Example notebook improvements

Various improvements for datashader example notebooks. So far just makes some small fixes to census examples but still working on revising network examples.

I agree that census-hv-dask should be renamed, but that name's a bit long.  How about just census-congressional?

@jbednar Are there any other notebooks you want me to go over? Otherwise this is ready to be merged, do note however that it the graph notebook depends on:
https://github.com/ioam/holoviews/pull/1951
and good performance on the congressional districts notebook depends on:
https://github.com/ioam/holoviews/pull/1952
https://github.com/ioam/geoviews/pull/90

Well, if you're asking, pretty much all of the notebooks need a going-over, but the main one that needs attention is landsat.ipynb.   Instead of InteractiveImage, it needs to use HoloViews with regrid (or, ideally, datashade, after making datashade do regridding as we discussed elsewhere).  That's important on its own, because it illustrates how to use raster data with HoloViews+Datashader, but also because it would make it possible to show the hyperspectral image data that's in those files.  I had a discussion with someone (I've now forgotten whom!) who was imagining having an image plot that when you  hover or click on it, will update an adjacent Curve plot with the full spectrum of radiant energy being measured at that location. Basically, we have 11 wavelength bands available, with a lookup chart at the top of that notebook listing what they are, and we can take any one of the image plots already in there and then for a given coordinate look up one number from that location in each of the 11 files to make a spectrum curve.  Seems like a cool example of HV, to me!

This could now be merged but it would be nice to switch the pcap example to use the inbuilt datashader forceatlas layout rather than relying on NetworkX. We'd need #485 to be fixed first though.

I'm ready to merge as soon as the above issue is addressed.  I think we should deal with forceatlas2 separately, preserving this version as a reference.  Thanks for all your excellent contributions here!


[Redesign] Inconsistency: package names are lowercase on /stats

Maybe this is intentional, but I'm not sure why it would be? On the /stats page the package names are lowercased. This was also true on the old design, but since we're giving it love, logging the inconsistency here.
Current look:

Related note: the top of this page is a really good place to also include a relative date. For example, I'd do this:

Statistics last updated at 2017-07-20 06:53:16 UTC (20 minutes ago).

/cc @joelverhagen

I'll look at the feasibility of using the package-author's intended ID casing. I'm pretty sure this is an artifact of our stats pipeline though and may take a lot of effort.

Ah that makes a lot of sense, please let us know either way! Totally understandable if this isn't a UI-only change, feel free to close it if that's the case (issue should go elsewhere if it is).


Definition of what is considered Critical vs non-critical

Is there any place where the difference between the two types of notifications are defined? What is considered a critical notification? It is unclear from the documentation exactly what notifications will be allowed by the "Hide Non-Critical Notifications" policy.

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

ID: 65d57fa8-9c9b-63cc-7248-175242963432
Version Independent ID: 9cd96c93-8f46-64db-a162-02da1e1f63cc
Content: Hide notifications from the Windows Defender Security Center app
Content Source: windows/security/threat-protection/windows-defender-security-center/wdsc-hide-notifications.md
Product: w10
GitHub Login: @andreabichsel
Microsoft Alias: v-anbic

@officedocsbot assign @mypil

@CCDFischer - Thank you for submitting feedback.
I will get this issue over to the Win10 ITPro writing team for investigation.
Thank you for reporting and making the docs better. Much appreciated.
I made a note to request the team to update this when the work is complete.

@CCDFischer in my opinion the easiest way to understand what is critical and what is not is to check the Security Log (and Application Log) under Event Viewer -> Windows Logs in Windows 10.
@bitslacker i will request a change to address that. Thank you for your feedback!

@VLG17 While you may be able to find that information in the event viewer, it is not documented anywhere what the difference is between these categories of notifications. As this is the documentation for the product I would expect to find at least an explanation if not an exhaustive listing here of what each category is. The point of documentation is that I should not need to discover this information through trial and error or log searching when clearly it is defined somewhere within MS. As such my request stands, please add in a section that at least explains what the types of alerts, and include examples if possible to make it easier for me and others to go to management and show them what will bee allowed and blocked with the difference GPO settings.

@dansimp - Can you please share your insights on this issue?
Thank you.

@Dansimp - We would like to follow up on this. Do you have any updates?
Thank you.

@Dansimp - We would like to follow up on this. Can you please share your insights on this issue?
Thank you.
cc: @AndreaBarr

@CCDFischer - The latest progress on the status is that the doc owner informed us that this topic will be updated.  Thank you.
cc: @Dansimp @AndreaBarr

We've addressed this with a PR in the private version of the repo. I recently added to the page a table listing which notifications are considered critical/non-critical -- it should go live soon.

@CCDFischer - From our understanding, the issue has been resolved. If you feel it hasn't been resolved, please re-open this issue.
Thank you for your contribution to make the docs better! Much appreciated!

@officedocsbot close


[WIP]Lazy init session storage and deprecate enable_session

Close #190

A new inspection was created.

The inspection completed: No new issues

Codecov Report

Merging #192 into develop will increase coverage by -1.52%.

@@             Coverage Diff             @@
##           develop     #192      +/-   ##
===========================================
- Coverage    78.77%   77.25%   -1.52%
===========================================
  Files           28       28
  Lines         1027     1042      +15
===========================================
- Hits           809      805       -4
- Misses         218      237      +19

Impacted Files
Coverage Δ

werobot/robot.py
76.77% <45%> (-4.35%)
:x:

werobot/session/init.py
70% <ø> (-20%)
:x:

werobot/session/filestorage.py
82.6% <ø> (-8.7%)
:x:

werobot/logger.py
44% <ø> (-4%)
:x:

werobot/utils.py
98.63% <ø> (-1.37%)
:x:

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 e567f08...37e2337. Read the comment docs.

A new inspection was created.

The inspection completed: No new issues

A new inspection was created.

The inspection completed: No new issues

A new inspection was created.

The inspection completed: No new issues

A new inspection was created.

The inspection completed: No new issues

A new inspection was created.

The inspection completed: No new issues


Support registering services against specific hosts

This fixes issue #2058

Ruby flake should be unrelated; Jenkins green.

LGTM otherwise


Add explicit on/off toggles to API

As a user I'd like more control over whether a macro is active or not. To do so, I would like to use functions to set the macro active:

Marking it active if it was inactive
Leaving it active if it already was active.

And likewise for setting a macro to inactive.

Added in v0.5.0 :)


SAGE with Metro-UI-CSS V3

Hi,
It's possible to add this : https://github.com/olton/Metro-UI-CSS
in my sage theme ?

As per the contributing guidelines this type of discussion goes on http://discourse.roots.io not the issue tracker


The fixes to issue #19

I hope I'm doing it right, it's like my second PR.
I changed read -ra args to read -r args and it seemed to fix all my problems

On my local computer and on gitpod when deploying contracts the code in the repository worked for contracts that only had 1 constructor.
Case 1. When there are no constructors parameters i.e. constructor() {}
What happened : Errored out as the command is forge create ./src/${Test}.sol:${Test} -i --rpc-url $rpc --constructor-args instead of forge create ./src/${Test}.sol:${Test} -i --rpc-url $rpc
Reason: the command expects the constructor to take in parameters that are supplied by --constructor-args. So there is the forge create that thinks the constructor has arguments as --constructor-args is passed to it.
Fix: Added an if statement to check if the parameters supplied to forge create for the constructor is 0 or not.
If 0 the forge create  command executes without --constructor-args.

Case 2. More than 1 constructor parameters i.e. constructor(uint256 a, string calldata b, ...) {}
What happened : read -ra stores the args passes as an array, but my terminal and gitpod use -rA instead of -ra so this could be a wider issue.
Reason : With read -rA ${args[@]} should be used for all the parameters to be passed in forge create making the command
forge create ./src/${Test}.sol:${Test} -i --rpc-url $rpc --constructor-args ${args[@]}
Fix: Using read -r instead stores all the parameters passed as a string that can be directly passed as ${args}

Interesting... Yeah... Do you know why your terminal isn't using -ra? It seems really odd that it's doing -rA. Using -ra should cover both of those since it'll just dump the args into an empty array.


SAK-43668: Grader WC > allow resubmission checkbox is too sticky

https://jira.sakaiproject.org/browse/SAK-43668
When the instructor uses the grader to grade and allows a resubmission for one student, then moving to grade the next student, the resubmission selection is kept. This is not how the "old" grading interface worked for assignments and I think it will cause errors and confusion for the instructor. In the "old" grading interface, the instructor had to set allow resubmission for each student individually if the option was not already set when creating the assignment.
JIRA Triage agreed this should behave the as the "old" grading UI,

Just noticed my IDE replaced a few == with ===. I think it's fine but let me know if I'm mistaken.


Help with getting the correct type while creating an instruction with buildCall

Hello, I was trying to create a instruction which requires "this" as an variable:

But so far I wasn't able to decide what I should place so that typeof() can convert it correctly, I've tried void but it only threw an error while saving the patch:

Any kind of help will be appreciated

Apologies didn't see the issue. Did you manage to figure it out?

yeah, I gave a look at the other issues and found what I was looking for, thanks anyway


Catch errors in ScriptEval to prevent crash of server

fixes https://github.com/miralemd/qlik-sse/issues/1

Issue addressed by @miralemd

@RobWunderlich I'm a bit hesitant on the async flag since that would kind of break the  FunctionDefinition. Luckily it's possible to detect if a function is async which I've added support for now.
Regarding the catch errors, I did start to refactor a few months ago when you initially created #1, so merging the PR would have caused conflicts for me - sorry about that:).

I like your async detection solution better. It does the trick.