Bulk upload creates corrupt file attachments if it encounters a rollback

Occasionally, we are seeing these exceptions in production:
 :type option required

/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/data_streaming.rb:121:in `send_file_headers!'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/data_streaming.rb:73:in `send_file'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/instrumentation.rb:30:in `block in send_file'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/notifications.rb:206:in `block in instrument'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/notifications.rb:206:in `instrument'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/instrumentation.rb:28:in `send_file'
/usr/src/app/app/controllers/submissions_controller.rb:156:in `show_correction'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/abstract_controller/base.rb:215:in `process_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/rendering.rb:53:in `process_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/abstract_controller/callbacks.rb:234:in `block in process_action'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/callbacks.rb:118:in `block in run_callbacks'
/usr/local/bundle/gems/actiontext-<IP_ADDRESS>/lib/action_text/rendering.rb:20:in `with_renderer'
/usr/local/bundle/gems/actiontext-<IP_ADDRESS>/lib/action_text/engine.rb:69:in `block (4 levels) in <class:Engine>'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/callbacks.rb:127:in `instance_exec'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/callbacks.rb:127:in `block in run_callbacks'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/callbacks.rb:138:in `run_callbacks'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/abstract_controller/callbacks.rb:233:in `process_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/rescue.rb:22:in `process_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/instrumentation.rb:67:in `block in process_action'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/notifications.rb:206:in `block in instrument'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/notifications/instrumenter.rb:24:in `instrument'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/notifications.rb:206:in `instrument'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/instrumentation.rb:66:in `process_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal/params_wrapper.rb:259:in `process_action'
/usr/local/bundle/gems/activerecord-<IP_ADDRESS>/lib/active_record/railties/controller_runtime.rb:27:in `process_action'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/abstract_controller/base.rb:151:in `process'
/usr/local/bundle/gems/actionview-<IP_ADDRESS>/lib/action_view/rendering.rb:39:in `process'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal.rb:188:in `dispatch'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_controller/metal.rb:251:in `dispatch'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/routing/route_set.rb:49:in `dispatch'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/routing/route_set.rb:32:in `serve'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/journey/router.rb:50:in `block in serve'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/journey/router.rb:32:in `each'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/journey/router.rb:32:in `serve'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/routing/route_set.rb:852:in `call'
/usr/local/bundle/gems/warden-1.2.9/lib/warden/manager.rb:36:in `block in call'
/usr/local/bundle/gems/warden-1.2.9/lib/warden/manager.rb:34:in `catch'
/usr/local/bundle/gems/warden-1.2.9/lib/warden/manager.rb:34:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/tempfile_reaper.rb:15:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/etag.rb:27:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/conditional_get.rb:27:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/head.rb:12:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/http/permissions_policy.rb:38:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/http/content_security_policy.rb:36:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/session/abstract/id.rb:266:in `context'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/session/abstract/id.rb:260:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/cookies.rb:704:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/callbacks.rb:99:in `run_callbacks'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/callbacks.rb:26:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/debug_exceptions.rb:28:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/show_exceptions.rb:26:in `call'
/usr/local/bundle/gems/railties-<IP_ADDRESS>/lib/rails/rack/logger.rb:40:in `call_app'
/usr/local/bundle/gems/railties-<IP_ADDRESS>/lib/rails/rack/logger.rb:25:in `block in call'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/tagged_logging.rb:99:in `block in tagged'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/tagged_logging.rb:37:in `tagged'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/tagged_logging.rb:99:in `tagged'
/usr/local/bundle/gems/railties-<IP_ADDRESS>/lib/rails/rack/logger.rb:25:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/remote_ip.rb:93:in `call'
/usr/local/bundle/gems/request_store-1.5.1/lib/request_store/middleware.rb:19:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/request_id.rb:26:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/method_override.rb:24:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/runtime.rb:22:in `call'
/usr/local/bundle/gems/activesupport-<IP_ADDRESS>/lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/executor.rb:14:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/static.rb:23:in `call'
/usr/local/bundle/gems/rack-2.2.7/lib/rack/sendfile.rb:110:in `call'
/usr/local/bundle/gems/actionpack-<IP_ADDRESS>/lib/action_dispatch/middleware/host_authorization.rb:131:in `call'
/usr/local/bundle/gems/prometheus_exporter-2.0.8/lib/prometheus_exporter/middleware.rb:41:in `call'
/usr/local/bundle/gems/railties-<IP_ADDRESS>/lib/rails/engine.rb:530:in `call'
/usr/local/bundle/gems/puma-4.3.12/lib/puma/configuration.rb:228:in `call'
/usr/local/bundle/gems/puma-4.3.12/lib/puma/server.rb:727:in `handle_request'
/usr/local/bundle/gems/puma-4.3.12/lib/puma/server.rb:476:in `process_client'
/usr/local/bundle/gems/puma-4.3.12/lib/puma/server.rb:332:in `block in run'
/usr/local/bundle/gems/puma-4.3.12/lib/puma/thread_pool.rb:134:in `block in spawn_thread'

Request Params
{"controller"=>"submissions", "action"=>"show_correction", "id"=>"[FILTERED]"} %>

We have not been able to reproduce them in development. Recently, a user gave us a hint as to what might be the issue here.
Namely, the user had (as a tutor) used the bulk upload for corrections. This failed because one of the included files exceeded the size limit, but some users still got a (somehow corrupt) correction attached. When they try to display them, the exception is triggered.
We should try to reproduce this in development. Possibly, the bulk_corrections! method in submissions.rb does not properly handle the case of a rollback of one single file in the bulk.

Maybe related: https://discourse.shrinerb.com/t/mime-type-nil-due-to-marcel-detecting-empty-file-when-it-is-not/210


Docs folder standardization and other fixes

README part on testing is now in internals so should be removed from readme and linked there.


401 on doc access

Describe the bug
Navigating to https://www.threatdragon.com/docs/development/env.html results in a 401.
Expected behaviour
Docs page to load
Environment

Version: N/A
Platform: Website (not TD Web App)
OS: RHEL
Browser: Chrome

To Reproduce
Use incognito to navigate to https://www.threatdragon.com/docs/development/env.html
Any additional context, screenshots, etc
Response:
{"status":401,"message":"Unauthorized","details":"You must login to continue"}

Many thanks for this @sparticvs - and indeed this link is broken. It is due to a recent change of name for this page to :
https://www.threatdragon.com/docs/development/environment.html
I will apply a fix for this so that www.threatdragon.com/docs/ will work again


Feature-windows_support

This PR should add linux support.
Environment:

Ubuntu 22.04

Changes:

Load TF Lite binary
update examples (same as macos, linux) to support windows

Notes:

Camera is disabled for the examples on desktop as this is not really supported

Progress:

[x] load TF Lite on windows
[x] test examples

[x] audio classification

uses some platform channels that do not work on windows

[x] image classification mobilenet

only from file works

[x] live object detection ssd mobile net

uses camera package that only works on mobile

[x] object detection ssd mobile net

only from file works

[x] style transfer

works

[x] super resolution esrgan

works

[x] text classification

works

Why this become draft sir?

Because it is not done.

@PaulTR This is now working for me locally.
One thing that should be added is how to compile the TF Lite binary on Windows. Following the procedure linked in the README I only get a debug binary. @PaulTR @moshOntong-IT any ideas?

@PaulTR many of the new examples added after the macos pr have quite a few problems when running flutter analyze .

@st-duymai Hey Duy, would you mind checking into the flutter analyze issue on the samples? This PR is also ready for testing it looks like.
Thanks!

Hi @PaulTR , @CaptainDario Since this project uses Melos to manage project, you must run dart pub global activate melos and melos bootstrap before flutter analyze .. Run flutter analyze . without melos can causes missing dependencies and locally linking package issue.

This PR is also ready for testing it looks like.

I will test it tomorrow on my Windows machine at home.

@PaulTR @CaptainDario I have tested all of the above apps, and they all work except for the features related to the camera and recording audio. Two projects, live_object_detection_ssd_mobile_net and audio_classification, do not work completely because they are not implemented for Windows. I think that we should remove the Windows package from these two projects.

@PaulTR @CaptainDario I have tested all of the above apps, and they all work except for the features related to the camera and recording audio. Two projects, live_object_detection_ssd_mobile_net and audio_classification, do not work completely because they are not implemented for Windows. I think that we should remove the Windows package from these two projects.

Sounds good. Yeah camera isn't expected to work for desktop (just isn't supported generally by Flutter from what I understand). I'm not sure if a similar limitation exists for audio - if audio can be supported, that'd be awesome, but I'm not too worried about it, so cleaning that up would be perfect and then we can get this merged.
Thanks for the heads on on Melos!
As for the binaries, I'm not sure if there's a difference in release vs debug ones? I'm only aware of the one set of docs for creating them. If there's something blocking with that though I can definitely bubble it up to eng.

With the audio example, I do not know how it works. It is calling some platform code that does not work on Mac, Linux, Windows. However, I added to each README which platforms are supported, I thought that should work.
Example for audio:

In regards to the binaries, for me the confusing thing is, that on Mac and Linux it does not create a debug binary by default. I think it is really confusing, but maybe that's only me.


Refactor/quality of life

To-do

[ ] Bump utils version when I cut a new release that package -- the latest version of dbt-utils uses a bad implementation of current_date, which will be fixed as part of the new release (see this PR). The new release will be cut on Wednesday or Thursday
[ ] Enable this repo on CircleCI and copy env vars over from utils integration tests (cc: @drewbanin)

Description
Major quality of life improvements. In particular:

Fixes bug when using custom generate_schema_name macro (closes #14)
Removes use of datediff in lookml code (closes #3)
Adds a warning regarding performance to readme (closes #15 (-ish)).
Tidies up some code
Adds issue templates
Adds integration tests for postgres, Redshift, and Snowflake, and then later make the tests more robust to account for the case when the audit schema does and does not exist.
Adds BQ compatibility (truly, I was poking in the dark here, but I think it closes #13 -- this may be a generally bad idea to add BQ compatibility due to API limits)

I just set up CircleCI to build this repo using the same CI creds as dbt-utils :)

Thaaaaank you @drewbanin -- I just snuck in BQ compatibility, can you copy over the env var for the service key? :pray:

I don't think we need/want to do that in CircleCI. You should supply this variable in the Circle config file as shown in dbt-utils.
This env var just points the BQ integration test profile to the keyfile written out in the ci job setup: https://github.com/fishtown-analytics/dbt-utils/blob/e2f5a718268a5e64e40f9fe7c33f346c3c635104/.circleci/config.yml#L13-L16

Hmm I have that part in my circle config too: https://github.com/fishtown-analytics/dbt-event-logging/pull/15/files#diff-1d37e48f9ceff6d8030570cd36286a61R13-R16


services/soroban-rpc: Support new XDR

Figure out how to build the new xdr into the go repo for this. We can keep existing services using the current xdr, but add a new next folder, and use it from the soroban-rpc service.
See: https://github.com/stellar/stellar-xdr-next for new XDR
New txn envelope and operation for ingestion
New ledger close metas, including SCVal types.
Complication
In particular, figure out implications not including transactionresults anymore, as per stellar-protocol/cap-0056.md at 2a3c395c4d838b616a4eda87a9ac4856f4ca5912 · stellar/stellar-protocol and comment

should only impact places we use a TransactionResult without the TransactionMeta available. Which is probably nowhere? do a grep in the codebase

Depends on https://github.com/stellar/go/issues/4558

@2opremio @tamirms , is this version built as a horizon unstable package yet, if not, is it ok if to kick off a build for 'soroban' pkg, I'm working on #4559 , updates to quickstart image, it needs to pull deb pkg of horizon with xdr support so that soroban-rpc can use the horizon api.

@sreuland sure, you can create a soroban build of horizon

i used 0.0.0~soroban version prefix, first build here stellar-horizon_0.0.0~soroban-299


exodus: init at 19.5.24

Motivation for this change
Initial packaging of exodus wallet.
Things done

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

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

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

@GrahamcOfBorg build exodus

@Lassulus I'm sorry, I did not realize dontBuild makes ofBorg to skip any build testing. I'm looking for a reviewer though, if you have time. :)

@mmahut I think ofBorg skipped things due to the unfree licence, not the dontBuild

@alexarice you are right!

The things I put about indentation might be github showing things weirdly for me now I've looked at it again

Builds and runs on my laptop and the indentation stuff was just github.
The desktop item also works

tested with nix-review and started it. seems to work.


Press and hold left mouse button will not fire events when right mouse button is used

Version

Phaser Version: 3.55.2

Description
Hold down left mouse button and click once right mouse button.
The right click is not detected.
After release the right button and then left button, no event of pointerup is fired for left button.
Example Test Code
Test it in the examples
https://phaser.io/examples/v3/view/input/mouse/right-mouse-button
Press & hold left key places an element, click right does than nothing.
update()-method detects the change (display rightButtonDown as text), but no event is fired.
Additional Information
When you try it with left click and e.g. wheel click or mouse button 4, it works.

Thanks, i tested it only in vivaldi, in chrome it works. So it is a browser problem.
Have to switch the browser for developement.

     }
     //Note that technically right and both OR left and both will always trigger. It is impossible to detect a simultanious press of both without first detecting one or the other.
   } );

  thingZone.on('pointerup', function (pointer) {
     if (pointer.rightButtonReleased() == true) {
        pinchRight = false;
     }
     if (pointer.leftButtonReleased() == true) {
        pinchLeft = false;
     }
     if (pointer.rightButtonReleased() == true && pointer.leftButtonReleased() == true) {
        pinchBoth = false;
     }
  } );`


Fixes #278 Added async versions of methods to IFtpClient

This pull request will add the async methods to IFtpClient.
This will fix #278

Tysm for this!

Thanks for accepting my pull request.
Would it be possible that you make a new release with this change included?

Yes, I'll make a release today.

I tried to build a release it but I'm getting errors with VS 2017 on my PC. The .NET 2.0 version is not building coz it needs a targeting SDK. Until that is resolved I cannot release this. If you can help me I would appreciate it. I'm using Visual Studio 2017 and need a targeting pack for .NET 2.0 but such a thing is unavailable from MS.
This is the error:

I finally managed to get a build going by manually building from VS 2017.

Since I built it manually, could you download the latest nuget and please inform if its all ok?

Everything seems to work! Thanks a lot!


[Feature Request] Support user-defined/script backup model

gobackup is great so I want to use gobackup to backup my postgresql as well. After some investigation I found that seems gobackup supports only full-backup for now, and it doesn't support Incremental Backup for postgresql database yet.
So i'm wondering if gobackup could support user-defined/script model. and then users can extend their-own backup script, it might be something like in gobackup.yml:
models:
  my_custom_app:
    backup_script:
      path: /path/to/my/backup_script.sh
    split_with:
      chunk_size: 100m
    storages:
      local:
        type: local
        keep: 10
        path: /data/backups
    archive:
      includes:
        - /data/home/git/my-beancount-template

It is better keep to support Splitter/Encryptor in such model.

Use before_script in v2.9.0


(IGNORE) Added missing "sample/" file prefix in Readme example

Changes in this pull request
Added missing "sample/" path prefix to the image.jpg file in the Generating an external manifest example.
Checklist

[x] This PR represents a single feature, fix, or change.
[x] All applicable changes have been documented.
[x] Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

Fixed with #200


Done button

Is there a way how can I add a Done button to REV? I mean if you type on UITextField, the original Apple keyboard shows up and there is a done button in right down corner. But REV has only return.

Ok, there is a solution for everybody:
http://stackoverflow.com/a/27079103


Resync 20240119

resync to u/s tag v0.27.8, plus cherry-pick all the NRT related changes between v0.27.8 and current u/s master branch

/hold
need to doublecheck for regressions because reverts + massive bump

/cherry-pick release-4.15

/hold cancel
local additional test passed


Treatment of non-height data channels

The problem
Many file formats support non-height information, for example the piezo voltage and others. Right now some readers return those channels (DI, IBW) while some do not (EZD, GWY).
The goal
Moving forward with the code base, I think we want to support non-height data channels and should implement reading those. Issues to resolve are:
Possible solution

We need to report two units - lateral unit and data unit. Suggestion: If information is not-height, then unit should be a tuple with lateral unit first and data unit second. This would not break most of the downstream codes.

Some properties have explicitly height in the names, e.g. the method heights() or the height_scale_factor. We should change the API to reflect this (maybe deprecating heights in the long run).

The channel index is currently used to uniquely refer to the data channel. Those indices would change if we just remove skipping of non-data channels in DI and IBW readers. This will break downstream topobank badly, because the channel index is stored verbatim in the database to refer to a specific channel. There should be a unique channel-indexing scheme that is invariant to the actual order of channels in the data file. Note that the channel name cannot be used as there are data files that have multiple channels of the same name. We could of course normalize that name by appending an index, but then the order matters again.

Some of the code is currently specific to height, e.g. unit name mangling. We should probably move this to a third-party library such as pint.

This requires #360 to be implemented


ENH: optimize.HessianUpdateStrategy: add __matmul__

Reference issue
This came up in https://github.com/scipy/scipy/issues/8792#issuecomment-1873466335
What does this implement/fix?
Allows to use the matmul operator @ for HessianUpdateStrategy.
Additional information
This is still missing a strict test that @ = .dot .
CC @jeffrey-hokanson : any help reviewing would be much appreciated :)

This is ready from my side. Last thing left: I am not sure how to document that __matmul__ is implemened or if we want to do that.

Thanks @dschmitz89. This LGTM overall, only a couple of minor comments.

Last thing left: I am not sure how to document that __matmul__ is implemened or if we want to do that.

A basic example using @ in the Examples section of the relevant classes seems like the way to do this to me.

Thanks for the review @rgommers . I looked into adding an example but as HessianUpdateStrategy is an abtract base class and neither BFGS or SR1 already contained examples, I did not add a dedicated matmul example. Instead I added it in the sentence listing methods. Hope that is also fine for you.


Added collapsible sections on news page

👷 Deploy Preview for balkantransport processing.
🔨 Explore the source changes: a69d575815ded8d32af0730f6ed46d1ed1e02c0f
🔍 Inspect the deploy log: https://app.netlify.com/sites/balkantransport/deploys/6188083d7ec7490008bbd0e1


Update README.md

Section:
Item:
Action: Add/Edit/Delete

Added three links to guides I've written related to Rocket Pool. Several in the community have already used these guides. I want to ensure others can easily find them.
By submitting this pull request, I promise I've read the contribution guidelines.

Sorry I didnt see this. LGTM, thanks


Allow Functions and Cmdlets written in PowerShell to inherit Parameters

For Cmdlets written in C# it is possible to inherit Parameters from other Cmdlets, but with the new features in recent PowerShell versions, more and more Cmdlets are written in PowerShell and it would be very helpful if Functions and Cmdlets written in PowerShell would be able to inherit Parameters from other Functions or Cmdlets.
This is especially important for Cmdlets interacting with REST endpoints which often have a large number of REST Commands which could result in several Cmdlets which share a common set of parameters.
This feature could be similar to [CmdletBinding()] which is already adding common Cmdlet parameters to a function.

@ffeldhaus  Can you provide some concrete examples of how you think this should look?  Also some things to think through...
There is a difference between inheriting from a class and inheriting from a command: type inheritance is done statically at compile time (even with PowerShell classes). Command inheritance would have to be done dynamically (i.e. at run time) so that implies dynamic parameters. In fact, using the existing dynamic parameter mechanism, you can (somewhat awkwardly) do this today. Take a look at this comment from @jleechpe  on a semi-related question from StackOverflow . The question is about mocking specifically but the code takes a function to mock and generates dynamic parameters based on the mocked function's parameters. This mechanism could be built into the runtime itself fairly easily. Note that this approach has two caveats: it will be slow relative to a straight function dispatch and it may be nondeterministic - the command you think you're inheriting from might not be the one the runtime finds.
An alternate approach that could resolve everything at compile time would be to have the command "inherit" from a class. What do you think about this approach?
Finally, what happens when there are the begin, process and end blocks in the base function. How do you see these being handled? Are they automatically called then the derived functions equivalent is called. Consider the case where the begin in the base function does some complex initialization with the parameters.

I think @ffeldhaus wants it in the case where you’re building a set of functions to access API calls. Since the parameters would mostly be the same being able to inherit would save code duplication (especially if changes needed to be made). I don’t think it’s a matter of calling the inherited function, just having it’s parameters made available (or at least I would have a use for it that way).
In my case it was for reports where I needed a set of criteria filters (first for an sql query and later for lists of machines) where the parameters were for different groups of devices or time spans.  I ended up needing the filter functions for multiple reports and was building out the functions for users who needed the actual call to be as simple as possible.
I had to recreate the parameters in each report (and then update them every time the sets of criteria were changed because of additional use cases), so I ended up using the dynamic parameters so I didn’t need to remember to update the 5+ primary functions (type of report + criteria) every time the business wanted a new criteria option.  Having a way to have a function inherit parameters and parameter sets natively would simplify the process.
The actual calls to the filter functions were as pipelines in the management function so the begin and end blocks processed fine.

Hmm... Well, could something like this be workable in terms of syntax?
param(
    [Parameter(ParentFunction = "Get-ChildItem")]
    $Path
)

One way or another, you'd still have to specify the parameter name, but this would allow you to 'borrow' the validation checks and any other attributes the parameter has in that function. I have very little idea of how parameter attributes actually work behind the scenes, but essentially PS should look at that attribute, go "OK, let's check Get-ChildItem for this parameter", take all its metadata from that function and apply it to the parameter here.
One concern I have with this type of 'inheritance' is that by design it can potentially obscure what the requirements of the parameter are. A potential method of mitigating that confusion is that the help for that parameter cannot be specified by the function; it is referenced and pulled from the parent function instead.

Something like that could work. However it wouldn’t help quite as much of you’ve got 5 or 6 variables, if not more, that you’d be doing that for.  If it were a similar construct that let you take all the parameters you’d still run the risk of obscuring the requirements, but if you only allow one inheritance from one function (and only of it’s parameters, not inherited ones (if any)) you’d at least limit the effects.

Hmm, yeah, that makes a lot more sense... So it'd probably look a bit more like... huh...
function Wrapper-Function {
    [CmdletBinding()]
    InheritParams('Get-ChildItem')
    #do stuff
}

That would be... a bit odd. It would also require changing probably a bit of the syntax checking logic around [CmdletBinding()] in the VS Code extension, because as-is if it's not directly followed by a param() block it calls it out as an error.
Not sure if that works better mimicking param() or as an attribute, but... hm. Interesting idea nonetheless.

I floated the idea in the Improvements for classes #6418 thread to allow us to create cmdlets with PowerShell classes like we can with C#. That would be a way to provide this functionality.

That would absolutely be the most effective way to get it done, I think. It would also make for an interesting addition in and of itself, being able to define C#-style cmdlets in native PS code...

I agree that being able to base Cmdlets on PowerShell classes would be the best way to get this implemented.
With C# there seems to be another way to share parameters between cmdlets by using Dynamic Parameters.

@ffeldhaus You can actually implement parameter sharing using dynamic parameters in script as illustrated in the rather long example below. The most significant line is this
 DynamicParam { Get-BaseParameters baseFunction }

Include this line in the derived function definition and you will "inherit" the properties from baseFunction. Here's the full example:
using namespace System.Management.Automation
using namespace System.Management.Automation.Internal

function Get-BaseParameters( $base )
{
    $base = Get-Command $base
    $common = [CommonParameters].GetProperties().name
    if ($base)
    {
        $dict = [RuntimeDefinedParameterDictionary]::new()
        $base.Parameters.GetEnumerator().foreach{
            $val = $_.value
            $key = $_.key
            if ($key -notin $common)
            {
                $param = [RuntimeDefinedParameter]::new(
                    $key, $val.parameterType, $val.attributes)
                $dict.add($key, $param)
            }
        }
        return $dict
    }
}

<#
    Function to inherit from
#>
function baseFunction
{
    [CmdletBinding()]
    param (
        [Parameter()]
            $Foo,
        [Parameter()]
            $Bar,
        [Parameter()]
            $Baz
    )
}

<#
    Function that extends the base function
#>
function derivedFunction
{
    [cmdletbinding()]
    param(
        [Parameter()]
            $Zork
    )

    DynamicParam { Get-BaseParameters baseFunction }

    End
    {
        [PSCustomObject] $PSBoundParameters
    }
}

derivedFunction -foo stuff -bar moreStuff -baz soThere -Zork "ZORK!"

The problem with the dynamic parameter approach is that they end up as second class citizens. Try running New-proxycommand on any of the AD cmdlets and you will see what I mean.
gcm gci,get-aduser | % {[System.Management.Automation.CommandMetaData]::new($_)} | fl parameters

I've run into this to when building a module wrapping p4.exe. It has the basic structure of
p4 [options] command [arg ...]
arg is unique to the command, but options are common to all, and it was not pleasant to try to maintain these.
I ended up moving to C#. Worked great for me, but maybe not what we want to recommend.

One question here is what prevents us from writing cmdlets in PowerShell classes?
That way you could write a base class with the common parameters.
Or some mix - not sure if this is a good idea - but letting a param block inherit from a class, or another function.
function CommonParameters {
	param(
		[Parameter()]
		[string] $Port
		,
		[Parameter()]
		[string] $User
	)
}

function Do-Stuff {
	param : CommonParameters(
		[switch] $Force
	)
}

PS> gcm -Syntax Do-Stuff

Do-Stuff [[-Port] <string>] [[-User] <string>] [-Force] [<CommonParameters>]

Inheritance of parameters is often insufficient - you often want to also share some code. This isn't hard to do with functions, but classes do provide a cleaner way.

Just to add to the mix here, there's also the pain point when you derive from a cmdlet base class that has parameters with parametersets defined. Sometimes you want to add new parameters to the sets in the derived class (or include base parameters in the derived sets), and you can't do this without overriding/shadowing the [Parameter] decorated properties and redefining a new parameterattribute. It would be nice to be able to do this more succinctly.

Bumping this!


switch to plug and play

No node modules -> less storage space used -> pog
Thanks yarn

We should also try this with schedule-personalizer, it would save file space

Schedule personalizer PR: https://github.com/insberr/schedule-personalizer/pull/92


Issue#21 GET retina scans

Description: Implement GET/view functionality of retina scans on patient profiles
11/09

Set retina photo display size to 256x256px

11/07

Created patient_images view in views.py to retrieve all retina photos associated with each patient + made them ordered by the most recent upload.
Updated view_patient.html to include a section that displays the retina photos along with other details like position(OS vs OD), date created, status, and prognosis. A message is shown if 'no retina photos are available.'
Tested on front-end; it works! but....The images are quite large. I need to resize them.


add rbac_id to appservice outputs

Adds the rbac_id output on App Service. Needed for Role Assignments.

hey @kottofy, I will integrate it as:
output "rbac_id" {
  value       = azurerm_app_service.app_service.identity.*.principal_id
  description = "The Principal ID of the App Service."
}

If you still want it to be azurerm_app_service.app_service.identity.0.principal_id you will have to put a condition so the output does not fail when there is no value
Error: Invalid index

  on ../modules/webapps/appservice/output.tf line 18, in output "rbac_id":
  18:   value       = azurerm_app_service.app_service.identity.0.principal_id
    ├────────────────
    │ azurerm_app_service.app_service.identity is empty list of object

The given key does not identify an element in this collection value.
Error: Process completed with exit code 1.

as per CI
Let me know if any issue with it :)


Run tests on a clean build before applying the new code changes

This is a feature suggestion: frequently I review code and I assume that the test is demonstrating the code change. However it happens that the test would be passing at head, which is usually a red flags: seems like having a warning to catch this can be useful.
(it just happened there: https://reviews.llvm.org/D71687#inline-651081 )

Let met try to understand the proposal:
When a user adds a new test case along with some other code, the CI system should cherry pick only the test case and try to run it on master branch. If the test compiles and succeeds there, the CI system should issue a warning.
Correct?

Yes that's the idea!
This may be difficult/fragile to implement though...


Test selective checks for SECURITY.md update

Check how much GH likes
::set-output name=needs-build::
needs-build=

Not very much: dev-support/ci/selective_ci_checks.sh: line 473: build_needed: unbound variable


Add spec coverage for explicit nil params vs unpassed params.

Signed-off-by: Dane Jensen<EMAIL_ADDRESS>

LGTM


Don't sort the results in the views

As it stands right now, smart_selects' views sort the result queryset lexicographically before sending it to the browser. This behavior cancels any custom ordering defined in the model's Meta.ordering.
You could stop sorting the queryset in the view, since anyone who wants the above behavior could specify so in Meta.ordering
This would break backwards compatibility for people depending on smart_selects' sorting, so it could be controlled by a setting.

Should be implemented in 3ffc87d0d99dc778c0ad3ce47608860820b7da86. PyPI release should happen later today or tomorrow. Ping me if I forget! 😄


BACKLOG-12813 Make header font size as on figma

JIRA

https://jira.jahia.org/browse/BACKLOG-12813
Description
Improve font size

It will be great to use the Typography component to manage font stuff.
SecondaryNavHeader component should be something like:
<Typography component="header"
                className={classnames(styles.secondaryNavHeader, 'flexCol_center', 'alignCenter')}
                variant="heading"
>
        {children}
</Typography>

To fit the design can also remove the font-weight of .variant_heading in Typography.scss


docs: addition of param and result sections

Improves documentation. (addresses #75)

Signed-off-by: Leonardo Rossi<EMAIL_ADDRESS>

ping @jirikuncar can you please review it? thanks


community[minor]: Update SingleStore vector store

Update the SingleStorevectorStore class with hybrid search functionality.
This PR contains:

Updated code (mostly, ported functionality that is already available in the python library)
Test
Updated document
New example

Hello, @jacoblee93! There was an automated comment regarding env variables access from examples and tests, in my mind this is fine, as it is just an example of a possible way of connecting to the database.
Also, should I change anything in the PR before it can be merged?
Thank you!

Hey @volodymyr-memsql - I'm getting src/indexes/vector_stores/singlestore_hybrid_search.ts(55,5): error TS2345: Argument of type '{ searchStrategy: "FILTER_BY_TEXT"; filterThershold: number; }' is not assignable to parameter of type 'SearchConfig' in one of the examples. Think you could have a look?

@jacoblee93, thanks for checking, I fixed the example code.

@jacoblee93, are any other changes needed for the PR?

Thank you!


Examples use the newly created analysis facilities

Use analysis instead of job where appropriate.

Looking good, thanks!


[DOC-3326] Update SMP air gap docs

Update SMP air gap docs
For reviewers, a preview is available.
What Type of PR is This?

[ ] Issue
[ ] Feature
[x] Maintenance/Chore

If tied to an Issue, list the Issue(s) here:

Issue(s)

House Keeping
Some items to keep track of. Screen shots of changes are optional but would help the maintainers review quicker.

[x] Tested Locally
[ ] Optional Screen Shoot.

Please check the Execution Link of the Pipeline for the Website Draft URL. This is located in the Preview Step and also is available in #hdh_alerts. E.g Website Draft URL: https://unique-id--harness-developer.netlify.app


[Story]: update scripts to run cypress tests in PRs

Goal
run cypress tests in PR checks
Dependency issue
TBD
once cypress is setup with valid tests
Itemized goals

include cypress tests with npm run test

Aspects continued elsewhere
No response
Mocks
No mocks

Done.


Update Presentation_project.md

Edited for English, and clarified a couple of points.

thanks a lot @Protohedgehog, let's hope to get people on board!
#1


Add the build target to the copy buffer after running bdc

After building locally, I find I often have to work with the target directory - having it in the clipboard would be really handy.
Published DataFrames, version 1.3.2 to /Users/jacobp/tmp/curriculum/DataFrames-1.3.2
and on the clipboard would be
/Users/jacobp/tmp/curriculum/DataFrames-1.3.2

Here's the problem: The build is running inside a Docker container, which doesn't have access to the host machine's (i.e., your Mac's) clipboard.

One possible solution is to have a “super-quiet” build mode that only outputs the build target directory on success. That way, you could just pipe the build command directly into pbcopy.


Added hxrsh.in to <512KB club

This PR is:

[x] Adding a new domain
[ ] Updating existing domain size
[ ] Changing domain name
[ ] Removing existing domain from list
[ ] Website code changes (512kb.club site)
[ ] Other not listed

[x] I used the uncompressed size of the site
[x] I have included a link to the GTMetrix report
[x] The domain is in the correct alphabetical order
[x] This site is not a ultra lightweight site
[x] The following information is filled identical to the data file

I confirm that I have read the FAQ section, particularly the two red items around minimal pages and inappropriate content and I attest that this site is neither of these things.

[x] Check to confirm

- domain: hxrsh.in
  url: https://hxsrh.in/ (Make sure you keep the trailing slash)
  size: 508
  last_checked: 2022-04-21

GTMetrix Report: https://gtmetrix.com/reports/hxrsh.in/VUiHMakp/

Very nice site, though the uncompressed size exceeds 512kb.😕 I'd love to see this on the list though. Maybe there's room for optimization somewhere?

Very nice site, though the uncompressed size exceeds 512kb.confused I'd love to see this on the list though. Maybe there's room for optimization somewhere?

Is this not uncompressed? The only metric the website shows is "page size", and I don't have an option to see if it's compressed or not. Where can I check uncompressed size?

@harshhhdev from the FAQ-section:

Do a GTMetrix scan on your website.
Once complete, click on the Waterfall tab to make sure the uncompressed size of your site is less than 512KB.

Your site appears to be 1.79kb uncompressed.


[Event Change Request] codeunit 7311 "Whse. Worksheet-Create" - event

Describe the request
We would like to handle the calculation of the "Qty. to Handle" field.
This event change happens in codeunit 7311 "Whse. Worksheet-Create" in the procedure AdjustQtyToHandle.
Current:
        ...
        if Location.Get(WhseWkshLine."Location Code") then
            if Location."Always Create Pick Line" then
                AvailQtyToPickBase := WhseWkshLine.CalcAvailableQtyBase(); // Set the Qty. to handle to the available quantity for "Always Create Pick Line" when transferring warehouse shipment lines to warehouse worksheet lines

        WhseWkshLine."Qty. to Handle" := TypeHelper.Minimum(AvailQtyToPickBase, WhseWkshLine."Qty. Outstanding");
        WhseWkshLine."Qty. to Handle (Base)" := WhseWkshLine.CalcBaseQty(WhseWkshLine."Qty. to Handle");
        ...
    end;

Requested:
        ...
        if Location.Get(WhseWkshLine."Location Code") then
            if Location."Always Create Pick Line" then
                AvailQtyToPickBase := WhseWkshLine.CalcAvailableQtyBase(); // Set the Qty. to handle to the available quantity for "Always Create Pick Line" when transferring warehouse shipment lines to warehouse worksheet lines

        OnBeforeSetQtyToHandle(WhseWkshLine, AvailQtyToPickBase ,IsHandled);
        if IsHandled then
            exit;

        WhseWkshLine."Qty. to Handle" := TypeHelper.Minimum(AvailQtyToPickBase, WhseWkshLine."Qty. Outstanding");
        WhseWkshLine."Qty. to Handle (Base)" := WhseWkshLine.CalcBaseQty(WhseWkshLine."Qty. to Handle");
        ...

Additional context
We would like to handle the calculation of the "Qty. to Handle" field.
Internal work item: AB#524418

This is already done. #26103


[Bug]: missing @babel/core in remix-eslint-config

Thank you for such a nice framework.
Which Remix packages are impacted?

[ ] remix (Remix core)
[ ] create-remix
[ ] @remix-run/architect
[ ] @remix-run/cloudflare-workers
[ ] @remix-run/dev
[x] @remix-run/eslint-config
[ ] @remix-run/express
[ ] @remix-run/netlify
[ ] @remix-run/node
[ ] @remix-run/react
[ ] @remix-run/serve
[ ] @remix-run/server-runtime
[ ] @remix-run/vercel

What version of Remix are you using?
1.1.0
What version of Node are you using? Minimum supported version is 14.
14.16.0
Steps to Reproduce

access https://stackblitz.com/edit/github-1ywvff?file=package.json
run npm run eslint

Expected Behavior
Eslint runs correctly
Actual Behavior
Error: Failed to load parser '@babel/eslint-parser' declared in '.eslintrc » @remix-run/eslint-config': Cannot find module<EMAIL_ADDRESS>
@babel/core is a peerDependency of @babel/eslint-parser.

Hello, is anyone working on it? If no, I'll take a look and try to do it.


kube-ui not in DockerHub

When trying to use the kube-ui addon, I'm getting the follow error when the pod is trying to start:
Failed	Failed to pull image "kubernetesonarm/kube-ui": Error: image kubernetesonarm/kube-ui:latest not found

In Readme:

kube-ui was removed, because the Kubernetes team shifted focus to dashboard.

It was a bit outdated and didn't fully work, so I removed it. Just kept the .yaml file for reference. Maybe I should write a README also in that folder about it.
But I have good news, dashboard is released soon and it's working on ARM also

@saturnism Now I've uploaded a snapshot of today's dashboard. So here's the Web UI
mkdir -p /etc/kubernetes/source/addons/dashboard
curl -sSL https://raw.githubusercontent.com/luxas/kubernetes-on-arm/dev/addons/dashboard/dashboard-rc.yaml > /etc/kubernetes/source/addons/dashboard/dashboard-rc.yaml
curl -sSL https://raw.githubusercontent.com/luxas/kubernetes-on-arm/dev/addons/dashboard/dashboard-svc.yaml > /etc/kubernetes/source/addons/dashboard/dashboard-svc.yaml
kube-config enable-addon dashboard

I tried it on a Pi 2 with latest hypriot and got the following error:

Strange, does it persist?
Could maybe try to build the image again...

well, yes - now I have another error while starting the pod:
kubectl logs kubernetes-dashboard-qei1f -p --namespace=kube-system
2016/01/29 12:39:49 Starting HTTP server on port 9090
2016/01/29 12:39:49 Creating API client for
2016/01/29 12:39:49 open /var/run/secrets/kubernetes.io/serviceaccount/token: no such file or directory
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x19304]

goroutine 1 [running]:
main.CreateHeapsterRESTClient(0x0, 0x0, 0x0, 0x113c47c0, 0x0, 0x0)
	/dashboard/.tmp/backend/src/github.com/kubernetes/dashboard/heapsterclient.go:36 +0xdc
main.main()
	/dashboard/.tmp/backend/src/github.com/kubernetes/dashboard/dashboard.go:49 +0x244

goroutine 17 [chan receive]:
github.com/golang/glog.(*loggingT).flushDaemon(0xfaab50)
	/dashboard/Godeps/_workspace/src/github.com/golang/glog/glog.go:882 +0x60
created by github.com/golang/glog.init.1
	/dashboard/Godeps/_workspace/src/github.com/golang/glog/glog.go:410 +0x2cc

From the message I guess, it tries to load a token and panics - will investigate further. If you guys have a quick fix, just tell me :)

@w0mbat Are you running this on a Pi 1? I've tested it on a Pi 2 only

no its pi 2 only. are you running hypriot?
where should this secret token come from?

Are you running a v0.6.3 cluster?

@luxas I used the hypriot 0.6.1 image and installed all available updates

You can try with the latest manifest that uses gcr.io/google_containers/kubernetes-dashboard-arm:v0.1.0. We built an official ARM binary with the dashboard team in the same way as I built kubernetesonarm/dashboard. Maybe it's working with that one @w0mbat?

Closing this as fixed. Removed kube-ui in favor for dashboard
@w0mbat open up a new issue if it panics with this version too


FIX ensure TOTP icon does not sit over the header

When verifying a TOTP code, the icon to the right of the input field was shifted
up and sat over the title. This is fine on the front-end, but in the CMS the title
is within a header on a modal, and this looked strange. It was also out of line
(haha, pun intended) with other authenticators and screens on that user flow where
all icons sat below the heading.
Removing the negative margin on the icon containing div solves this, an also makes
up for the removal of the form-group class that was adding an undesired divider
line between the input field and the actions, via silverstripe/admin applied styles
(when in the CMS - via admin/myprofile). This change removes spacing between the
input and the next button (et. al.), but the image moving down re-introduces the
spacing by forcing the parent container to be a larger height (that the class
mfa-totp__validate-left does not fill).
Resolves https://github.com/silverstripe/silverstripe-mfa/issues/295
Note that if tested this will look weird at small (mobile) viewport widths. I have not concerned myself with this as the fix in #31 and related issues on other repos addresses (it adds padding-top to the actions container at mobile widths).

This removes the action list alignment, since form-group previously had a bottom margin on it. Note that the backup codes screen still uses form-group. I'll make another PR to sort this stuff out.


Improvement onto icon images: Added ARIA labels to social media icons

Purpose
Currently when VoiceOver reads the social media icons in the footer of the OSF pages it just reads "link" for each of them. It has no idea what they are and to a blind or visually impaired person this could be a major problem. This PR allows VoiceOver to read what social media sites the icons represent and relay that information to the user.
Changes
Adds ARIA labels to the five icons listed under the header "Socialize" in the footer of the OSF. The labels simply state exactly what social media website the icon represents. For example, the icon for twitter has the ARIA label : "Twitter" . An attached image showcases what this looks like in VoiceOver. Additionally, as the label is an ARIA label it would not be visible to sighted users and would only become audible and visible to users using VoiceOver to access the OSF.
Side Effects
As only 5 lines of code were changed and of those changes only ARIA labels were added, no side effects are anticipated.

Sweet!

Was this associated with a JIRA issue?

This was not associated with a JIRA Issue that I know of.

I created the following JIRA issue to have it follow the dev process and be available for QA's queue.
https://openscience.atlassian.net/browse/OSF-5856

I wouldn't worry about it.  We normally have the issue in JIRA before the pull request and it is in the commit message where you would mention the JIRA number.
I commented with the JIRA number (as well as commenting on JIRA with the PR number) for clarity with QA.


Is it possible to change tx power ?

Hello,
It is more a question than an issue but i would like to know if it is possible to change the TX Power through the Mbed LoRa API ?
I checked the API and didn't found something that allow me to do this from the application code.
By the way, what is the default TX Power ? +27 dbm ?
Thank you,
Pierre-Marie

ARM Internal Ref: IOTCELL-1155

@pmancele There is no API to change the TX power as the TX power will be changed in response to a Mac command sent by the network server. This can happen either in LinkADRRequest mac command or TxParamSetupReq mac command. For network optimization, it is vital.

Ok thank you for the explanations !


Update to the latest commit

This package is not updated in almost a year. Let's update it.

!testme


Fixed size big integers?

Should BigInt receive a maximum bit-size parameter? Should we support some fixed size integers like u256? Are there crates that can provide some of this functionality for us automatically at reasonable performance/size?
Related to https://discord.com/channels/897514728459468821/1007337480179171441

I think we need bitsize caps for interior, otherwise developers will need to check for overflow themselves, which will lead to errors.
I think it's reasonable for us to add a bit size to BigInt<BITSIZE = ...> with a reasonable default value. We would need to add the bitsize to the XDR so that the aspect of the type carries into inputs, etc, and so the host can validate.
This approach has a few benefits:

BigInts of different max bitsizes aren't implicitly interoperable from the compilers pov, but we can have fallible conversion functions, mirroring Rusts approach to its own integer interactions.
We can type alias BigInt<256> to U256.
Users can use other integer sizes if they need, and we don't special case code using one size over others.

There are downsides:

While we can add functionality to BigInt for specific sizes like U256 if we need to, we can't specialize functions shared by all sizes. If we really want specialization though we could macro generate impls for a range of useful bitsizes and then hand code specifics.

Somewhat related to this we probably also need an unsigned version of BigInt.

Why can't this be done at the sdk level? Similar to how the sdk vec takes in <T> to ensure all elements are of the same type?

It could, but that's adding a lot of logic to WASM files that could be simply be applied at the host layer. Vec is a bit different, since T can be a complex type. With BigInt the constraint is well defined.
Is we're to go ahead with this I suggest we list out what the SDK interface is, then see what needs changing in the host to support it. Or, we could do as Jay suggests and start by implementing this in the SDK only to start as an experiment.

No longer relevant because we removed BigInt support.


Can not display this pdb file

Hi,
Original issue is from here (cc @kuenzeln).
Both nglview and NGL from website show nothing (although I was trying to change to licorice or line repr). VMD is showing ok.

Here is the file
ATOM      1  CA  GLY A   1       1.499 -33.933  11.693  1.00  8.98           C
ATOM      2  N   GLY A   1       2.255 -34.674  10.709  1.00  8.87           N
ATOM      3  C   GLY A   1       1.397 -34.742  12.954  1.00  8.66           C
ATOM      4  O   GLY A   1       1.731 -35.910  12.936  1.00  8.13           O
ATOM      5  N   SER A   2       1.970 -34.016  13.926  1.00  9.28           N
ATOM      6  CA  SER A   2       1.953 -34.477  15.356  1.00  9.37           C
ATOM      7  C   SER A   2       1.481 -33.344  16.231  1.00  9.54           C
ATOM      8  O   SER A   2       1.700 -32.210  15.911  1.00  9.78           O
ATOM      9  CB  SER A   2       3.355 -34.863  15.794  1.00  0.00           C
ATOM     10  OG  SER A   2       3.795 -36.004  15.054  1.00  0.00           O
ATOM     11  N   GLY A   3       0.793 -33.668  17.315  1.00 10.35           N
ATOM     12  CA  GLY A   3       0.549 -32.747  18.430  1.00 10.71           C
ATOM     13  C   GLY A   3       1.584 -31.628  18.574  1.00 11.53           C
ATOM     14  O   GLY A   3       1.194 -30.467  18.669  1.00 12.06           O
TER      15      GLY A   3
ATOM     16  CA  GLY B   1      31.499 -33.933  11.693  1.00  8.98           C
ATOM     17  N   GLY B   1      32.255 -34.674  10.709  1.00  8.87           N
ATOM     18  C   GLY B   1      31.397 -34.742  12.954  1.00  8.66           C
ATOM     19  O   GLY B   1      31.731 -35.910  12.936  1.00  8.13           O
ATOM     20  N   TYR B   2      31.970 -34.016  13.926  1.00  9.28           N
ATOM     21  CA  TYR B   2      31.953 -34.477  15.356  1.00  9.37           C
ATOM     22  C   TYR B   2      31.481 -33.344  16.231  1.00  9.54           C
ATOM     23  O   TYR B   2      31.700 -32.210  15.911  1.00  9.78           O
ATOM     24  CB  TYR B   2      33.340 -34.899  15.840  1.00  0.00           C
ATOM     25  CG  TYR B   2      33.809 -36.101  15.055  1.00  0.00           C
ATOM     26  CD2 TYR B   2      33.531 -37.392  15.519  1.00  0.00           C
ATOM     27  CD1 TYR B   2      34.521 -35.924  13.863  1.00  0.00           C
ATOM     28  CE1 TYR B   2      34.956 -37.038  13.135  1.00  0.00           C
ATOM     29  CZ  TYR B   2      34.678 -38.329  13.600  1.00  0.00           C
ATOM     30  CE2 TYR B   2      33.966 -38.506  14.792  1.00  0.00           C
ATOM     31  OH  TYR B   2      35.101 -39.411  12.893  1.00  0.00           O
ATOM     32  N   GLY B   3      30.793 -33.668  17.315  1.00 10.35           N
ATOM     33  CA  GLY B   3      30.549 -32.747  18.430  1.00 10.71           C
ATOM     34  C   GLY B   3      31.584 -31.628  18.574  1.00 11.53           C
ATOM     35  O   GLY B   3      31.194 -30.467  18.669  1.00 12.06           O
END

This has to do with the initial representation. Happens the same with di-ala. NGL notices that the molecules are made up of amino-acids, so it decides it is a protein, so it forces the use "NewCartoon" for them. The peptides are too short, no secondary structure is recognized so you see nothing. Can you try to add "add_licorce"  (or sth, I never remember) after the widget is instantiated. I have wrapper around nglviewr s.t. if n_residues is smaller than 10, "NewCartoon" should be deleted and "Licorice" used instead

ok, scratch that, it is not what I thought...

it seems it is not automatically centered properly, do a 'autoView()' after adding a licorice representation for a workaround

indeed.


Implemented program to find the smallest three elements in an array in C#

Added FindTheSmallestThreeElementsInAnArray.cs (codinasion#5168)

@anandfresh Added Issue ID to the PR.


Reset Wunderground alerts on API update

Description:
The wunderground web service may return multiple alerts if they are applicable. In that case, each of the sensor attributes is suffixed with the warning type. But since the attribute list is never re-initialized, values that disappear never get removed from the list.
This PR fixes the issue by re-initializing the sensor attribute dictionary for every API call. This is no problem because we never get differential updates. Instead, weather data is always returned completely.
Notes:

I added a test for this new feature. However, I don't know how to disable the Throttle in https://github.com/keneanung/home-assistant/blob/abebfbe554f7f236b63ae6ebcf9379a575b7d626/homeassistant/components/sensor/wunderground.py#L780 for the tests. It worked locally because I removed the line completely when developing the test. Can you advice what to do?
I will take care of the code style issues soon. I wanted to get the PR in to get a comment on the first point above. Done

Checklist:

[x] The code change is tested and works locally.

If the code does not interact with devices:

[x] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
[x] Tests have been added to verify that the new code works.

You need to fix all tests.

@fabaff I know the tests are not passing. As I wrote in the OP and is maybe evident through the last few commits, I don't know HOW. The throttled updates from the mocked API get in the way.

Wunderground has been converted to async in #12385. Is this PR still valid and needed? Please fix the merge conflicts.

This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it.


Use the Rust test framework

For the moment we use a custom test engine but we should investigate the use of the Rust built-in test framework. However I am not sure that it is suited for our needs.

Need to wait more support for testing syntax extension, close this for now but we will closely follow the evolution of Rust internal API.


Hanging GET on snapshots/analyse/:id endpoint when first requesting snapshot analysis

The flow for fetching analytics goes as follows:

If analytics have been processed before -> Fetch existing record

Else -> Compute analysis metrics and return once calculated (which may take an uncertain amount of time)

Web app is then waiting on this request to complete and sets the analytic state once fulfilled (which may take multiple minutes ++)
I hope I'm understanding this correctly and please correct me if I'm wrong. But my thoughts are:
I don't think this workflow is very user-friendly and I think we should not leave the request open when the analysis is processing. Instead we should just return something like 'status': 'In Progress' while its processing.

Addressed in #35


Document how to add "job-function" managed iam policies to roles

Is your feature request related to a problem? Please describe.
We had to go thru trial and error to add "Billing" managed iam policy to a role via roleSets. First we tried adding awsManaged: Billing, then the arn awsManaged: arn:aws:blah:blah:policy/aws/job-function/Billing (i know the arn is incorrect), and finally got it to work with job-function/Billing. This should be documented here: https://awslabs.github.io/landing-zone-accelerator-on-aws/classes/_aws_accelerator_config.PoliciesConfig.html#awsManaged or included in more examples. AWS recommends using these job function managed policies.

Hi @atheiman , thanks for sharing this. My pipeline will fail soon because I also didn't add the job-function.
policies:
 awsManaged:
 - AdministratorAccess

Should this be:
policies:
awsManaged:

job-function/dministratorAccess

For assumedBy section, which type do you use? I am trying type Account but unable to give a proper principal name. Similar to your case, there is no example.

This holds true with service policies, as well. For example, the AmazonDMSVPCManagementRole policy's arn is: arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole.
The documentation for the IPoliciesConfig interface currently shows that the awsManaged key accepts:

List of AWS managed policies. Values can be policy arn or policy name

This doesn't appear to be accurate. When specifying the policy name, it does not find the policy due to the IAM path. Specifying the arn, I received this error:

Resource handler returned message: "ARN arn:aws:iam::aws:policy/arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole is not valid.

It doesn't seem to recognize that an arn is being specified, resulting in the full arn being appended to the prefix for Policy arns. Specifying the policy name with the path included (service-role/AmazonDMSVPCManagementRole) seems to work.


singleArchFiles minimatch doesn't work for files in .webpack folder

By default, Webpack puts its files in a folder called .webpack. Then Electron Forge bundles these files into your app package.
However, without the "dot" option, minimatch ignores any files in this "hidden" directory. This means I get errors like the following:
[FAILED] Detected unique file ".webpack/main/index-x64.jsc" in "/var/folders/y2/m0k2q_mn2_jb9tm37k1wds1r0000gn/T/electron-universal-NzIpvm/Tmp.app/Contents/Resources/app.asar" not covered by allowList rule: "**/*.jsc"

I believe this can be fixed by adding dot: true to the minimatch options in checkSingleArch() function in asar-utils.ts.

I've solved my error by switching my rule from **/*.jsc to *.jsc which works because matchbase is enabled. But the above still feels like a bug.

I think that'd be functionally identical to {.*,*}/**/*.jsc with dot: false as well. I personally would prefer to keep dot: false since it's the default for minimatch.


Sqlite - Curious in an example adapter?

Hello all,
I'm making a sqlite adapter for a project. Would you guys be interested in using it as an example adapter? I can toss it in a pull request if you're interested. We chose sqlite because it's small, simple, easy, and good for random little projects, hopefully we can help out others.
Interested?

Yeah the more the merrier!

Closing awaiting a PR

I totally forgot about this sorry. I'll see if I can get something together.


chore: Add InsecureCredentialsWrapper for Emulator connection

A PR raised here to deprecate this class in the Core here: https://github.com/googleapis/google-cloud-php/pull/7089


Added missing .ConfigureAwait(false) (#7013)

Can one of the admins verify this patch?

See also https://github.com/Azure/azure-sdk-for-net/pull/6976

@mllab-nl @AlexGhiondea maybe you can also look at https://github.com/Azure/azure-sdk-for-net/pull/7014#issuecomment-516386535 which points to https://github.com/Azure/azure-sdk-for-net/pull/6976. It contains the necessary ConfigureAwait(false) calls for the whole lib. It also takes into account that the existing approach doesn't omit the state machine

/azp run

Test failures are unrelated.


@swc/core-win32-x64-msvc doesn't load inside node v18.15.0

Describe the bug
@swc\core-win32-x64-msvc doesn't load in node v18.15.0. It loads fine in node v18.14.2.
@swc/core@npm:1.3.42 STDERR Error: The specified module could not be found.
@swc/core@npm:1.3.42 STDERR<EMAIL_ADDRESS>@swc/core@npm:1.3.42 STDERR     at Module._extensions..node (node:internal/modules/cjs/loader:1338:18)
@swc/core@npm:1.3.42 STDERR     at Module.load (node:internal/modules/cjs/loader:1117:32)
@swc/core@npm:1.3.42 STDERR     at Module._load (node:internal/modules/cjs/loader:958:12)
@swc/core@npm:1.3.42 STDERR     at Module.require (node:internal/modules/cjs/loader:1141:19)
@swc/core@npm:1.3.42 STDERR     at require (node:internal/modules/cjs/helpers:110:18)
@swc/core@npm:1.3.42 STDERR     at Object.<anonymous> (c:\h\jenkins\workspace\xxxx\node_modules\@swc\core\binding.js:68:41)
@swc/core@npm:1.3.42 STDERR     at Module._compile (node:internal/modules/cjs/loader:1254:14)
@swc/core@npm:1.3.42 STDERR     at Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
@swc/core@npm:1.3.42 STDERR     at Module.load (node:internal/modules/cjs/loader:1117:32)
@swc/core@npm:1.3.42 STDERR     at Module._load (node:internal/modules/cjs/loader:958:12) {
@swc/core@npm:1.3.42 STDERR   code: 'ERR_DLOPEN_FAILED'
@swc/core@npm:1.3.42 STDERR }
@swc/core@npm:1.3.42 STDERR @swc/core was not able to resolve native bindings installation. It'll try to use @swc/wasm as fallback instead.

Input code
node -e 'require("@swc\core-win32-x64-msvc")'

Config
No response
Playground link
No response
Expected behavior
native module loads
Actual behavior
native module doesn't load
Version
1.3.42
Additional context
No response

<EMAIL_ADDRESS>works fine on linux with node v18.15.0

@viceice this is not a bug of SWC nor Node.js, ref: https://github.com/npm/cli/issues/4828

@Brooooooklyn It's not a npm issue, because the module is there. I verified that file exists before.  I'm using yarn v3+ to install with node_modules linker

Use this minimal test file
{
  "name": "test",
  "version": "0.0.0",
  "private": true,
  "dependencies": {
    "@swc/core-win32-x64-msvc": "1.3.42"
  }
}

npm i --no-package-lock
node -e 'require("@swc/core-win32-x64-msvc")'

@viceice what are you expected from node -e 'require("@swc/core-win32-x64-msvc")' ?
You should try node -e 'console.log(require("@swc/core-win32-x64-msvc"))'

node -e 'require("@swc\core-win32-x64-msvc")'

this is invalid, \ should be / in package name

yes, sorry. it's not a swc error, but a windows docker issue. please see the linked nodejs issue.

the solution is to install the Microsoft VC++ libs.
not sure if the is an option to detect the missing libs on a new plain system


Unexpected Error while trying to create a shortlink with query parameters

I think the title says it all. I checked the logs of the docker containers and the API response but I can't additional info for that error...
Is this just not supported, or are you planning on implementing it? The same error also occures random while creating shortlinks for seeminly no reason. Is there anything I can do to get a more detailed error msg?

It also happens when editing a valid shortlink and adding query parameters to the redirect url. I opened up DevTools on the web app and found that there is a POST error: 400 Bad Request. It happens in main.js in line 190. It is obfuscated but I'm gonna investigate this issue further.

I've found some time to look into it:
The problem is the URL Regex, which checks the destination URL. For some reason, it rejects URL queries. To be honest, I've copied this regex on some web page when I was googling for good URL regex strings.

This is the current destination validation regex:
^(ht|f)tp(s?)\:\/\/[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\.\?\,\'\/\\\+&%\$#_]*)?$

So I just found the folliwing code in LinkUtil.cs:
public static async Task<bool> ValidateDestination(string destination)
        {
            if (!UrlRegex.IsMatch(destination))
                return false;
            using (var client = new HttpClient())
            {
                client.Timeout = TimeSpan.FromSeconds(10);

                try
                {
                    var res = await client.GetAsync(destination, HttpCompletionOption.ResponseHeadersRead);
                    return (int) res.StatusCode < 400;
                }
                catch
                {
                    return false;
                }
            }
        }

So I checked the RegEx and found out, that it doesn't fully cover URL support. When i use query parameters it cannot match because = is not allowed. This is crucial for URL Query Parameters though.
Sidenote: https://google and https://google..de are valid URLs according to your RegEx (not that fixing it would matter as it would timeout anyways but could make the code more efficient in the future). Was that intended if yes why?
Your RegEx:
^(ht|f)tp(s?)\:\/\/[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\.\?\,\'\/\\\+&%\$#_]*)?$

My updated version to fix the issues above:
^(ht|f)tps?:\/\/(\w*\.)?\w+(\.\w{1,24})(:\d*)?([-\w()@:%_#\/]*)(\?[-\w()@:%_\+~#\/=&]*)?

I tested it on https://regex101.com and it seemed to be working fine.
Just update it in Contants.cs and this error shouldn't be happening anymore

I've found some time to look into it:
The problem is the URL Regex, which checks the destination URL. For some reason, it rejects URL queries. To be honest, I've copied this regex on some web page when I was googling for good URL regex strings.

This is the current destination validation regex:
^(ht|f)tp(s?)\:\/\/[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\.\?\,\'\/\\\+&%\$#_]*)?$

oh didn't spot it haha. Yeah that was what i found as well. Copying regex is not good practice :D
But this bug should be solved then.
Note the regex that I provided doesn't work for advanced URL syntax like<EMAIL_ADDRESS>


Global: Add Global English language

Plus, think of better name for it.
CC @sheydari-ds @mikefantini

@sergii-tkachenko sorry can you please clarify why you think we need a better name for it than "Global English"?
The idea is that the content that would be in those fields would meet two criteria:

they are in English
all information is globally relevant

The only distinguishing factor between this and US English is that US English would have some content that is more locally specific (i.e. the US English version of a campaign would have a stat like "1 in 3 teens in the US..." while the Global English version of a campaign would have a stat like "1 in 4 teens in the world...").
The idea is then when we want to serve Spanish speaking teens across the world we would have a Global Spanish translation. The only difference between the Global Spanish translation and the Mexican Spanish translation would be that the Mexican Spanish translation would have information specific to Mexico (i.e. "1 in 3 teens in Mexico") while the Global Spanish version would have information that worked in a global context (i.e. "1 in 4 teens in the world").


chore(master): release 1.2.0

:robot: I have created a release beep boop
1.2.0 (2024-02-04)
Features

updating dependencies (20a8a03)

Bug Fixes

labeler ci flow (b922790)

This PR was generated with Release Please. See documentation.

:robot: Release is at https://github.com/wayofdev/laravel-webhook-client/releases/tag/v1.2.0 :sunflower:


Can I get a link in the wiki?

I just released this and would like a link in the wiki:
https://github.com/burtonator/polar-bookshelf
https://www.inputneuron.io/polar-bookshelf-a-cross-platform-ebook-system-for-incremental-reading-and-spaced-repetition/

I just added the link to your project to https://github.com/mozilla/pdf.js/wiki/Gallery-of-user-projects-and-modifications. I think it's a nice idea and it's really good to see another useful custom deployment of PDF.js. Thank you!


Implement cycle checks for serialization

In the case where we dump local variables and there are nested references to a shared object, we will end up serializing the nested loops multiple times. We've implemented a max depth option which is good but not a complete solution. Other SDKs implement cycle checking in order to detect if an object has already been serialized.
cc @rokob

This has been implemented in PR #417


Remove reference to the "sign up form" because Az Deployment Manager now in Public Preview

Azure deployment manager no longer requires users to request access via the signup form according to this announcement: https://azure.microsoft.com/en-us/blog/announcing-service-monitor-alliances-for-azure-deployment-manager/
If true, please remove the following line from the top of the page -> "Azure Deployment Manager is in private preview. To use Azure Deployment Manager, complete the sign-up form."
Thanks.. JD

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

ID: 5d308d76-615c-ccae-6a73-545d513cb6f6
Version Independent ID: f75fd25d-9f75-c5ad-c82b-2151c20ff5b3
Content: Safe deployment practices across regions - Azure Deployment Manager
Content Source: articles/azure-resource-manager/deployment-manager-overview.md
Service: azure-resource-manager
GitHub Login: @tfitzmac
Microsoft Alias: tomfitz

@julesman101 Thanks for the feedback. We are looking into this and will get back to you shortly.
@tfitzmac FYI.

@julesman101 - thanks for making us aware of this needed change. I have removed the sentence.


trying to fix CI

still had to force re-run it once... not understanding why... but at least it fully worked


Comments don't add immediately, visitors count 0 permanently

As stated above, here are the bugs I've encountered:
1.When adding a comment it does not show until I've refreshed the page.
2.Visiting a profile still leaves the view stat at 0.
cheers mate I hope u can fix it :D

I would need a URL to test the first one, or at least the browser you've tested on.
As for the second issue, could you please tell me if you are visiting your own profile? Or maybe you're visiting someone else's profile repeatedly? Because it doesn't increment the number of viewers each time you refresh the profile.

Hello,
such a great plugin, this is what my board needs :D
However, I am also experiencing same problem with this asker.
So the problem is, after I have finished writing comment, I submitted it, but it was like nothing happened. Until I refreshed the page, my newly written comment appeared.
In my case, here is my board URL: http://www.noblun.com
Good luck great people!


Refactor Django ORM

I was hoping to refactor some of django_orm into django_util. This also ties into #319. There are currently 3 classes:

CredentialsField -  It makes sense to me for this to exist if you want to store the oauth credential as part of some sort of UserModel. You sometimes interact with the credential directly, and  you might want to use it to do an oauth call in a background process outside of the session context, for example, in which case you want a credential in the database rather then only in a session.

Storage - This is a storage class that uses the CredentialField as the storage object. This will be one of the Storage options for our helpers to use.

The one that I think we should maybe get rid of is

FlowField - Back when people had to roll their own auth flow, this would be one way to store the Flow between the user being redirected to the authorization server and the callback being called with the code to obtain the credential. But now we are providing the code to do do that for you, and using the session seems pretty reasonable given the entire flow should happen within the context of a single session. The only counter-example I can think of is if you are trying to use cookie-based sessions and can't fit the credential into the session (this is currently biting us for our Flask example app, but I think the solution is just to use server-side sessions). Even if we want to keep it, we should probably just make it a configurable option for our code to use the ORM to store the flow rather then the session.

My inclination is just to remove FlowField but I'm not sure if people use it. I guess it comes down to whether people are still rolling their own flows and if we need to still support those use cases, even though we're now providing the view logic to do it for them.
What do people think?

I am all for removing code. That entire file seems ill-conceived. I had a hard time figuring out many of the design decisions when I tried to test it a while back.

@waprin can we close this in favor of the new plan for Django stuff?

This is basically the tracking issue for that work, although the scope of changes is somewhat bigger then just killing that field.

Retitle this issue to better summarize the intended improvements?

Retitled, basic plan is:

[ Create usage docs to explain the two main use cases (oauth2 (Google) based user system vs adding oauth2 credentials into existing Django user system. ]
[Kill FlowField, Move CredentialFiield into django_util/models.py]
[Keep existing decorator usage exactly as is, add a new config option that lets you specify a Model with a Django UserFIeld and CredentialField, and make the decorator store credentials in that field (where request.user matches the model's user) ]
[Figure out best place to add complete canonical samples for both main use cases, either directly in this repo or more likely linked to]

I know this is closed and I'm late to the game, but I'm not sure this was thought through. I can only relate my experience, not that it matters as its too late but still might be helpful:

Django migrations that used the FlowField now error out. Apps won't even start until those older migrations are updated to be updated or removed. This kind of defeats the purpose of having those migrations because they were meant to rebuild the database change after change.
My application does not use sessions. The back end is API based, using tokens, and there is no real guarantee two subsequent requests will hit the same physical machine. The FlowField in my case was vital to store the flow between oAuth steps. I'm sure this practice is not unique to me. I'm not that special :)
Samples and docs are almost non existent - None offer a migration/upgrade path. This would have helped. The Google documentation still refers to FlowField and basically 2.x library constructs.

Many thanks regardless. I'll figure all this out - just thought its good to mention.

@harel thanks for feedback, some of it could have been better. Seemed like it needed updating, I tried to gather feedback on how it was being used but didn't get much. Now your explanation of how you're using things makes sense, I shouldn't have killed FlowField.
There are docs and samples in this repo's docs.
Apologies if it caused you any pain.

Tha'ts fine @waprin. For posterity, what i did was simple enough and can be replicated by anyone:

I migrated the FlowField to a Text field
Then, I manually edited the original migration for the model that stored my FlowField to be a Text field. This stopped the new migration pre checks from failing.
I added the pickle code lifted from the original FlowField into my model formerly known as the "model that contained the FlowField"
Changed all refs to the django_orm pacakge, removed FlowField from imports and change Storage to the new DjangoORMStorage.
I sat back, enjoyed the sunset and congratulated myself on a job well done while sipping gin and juice.

All is good.


Update index.html

Fixes Issue

Changes proposed

Screenshots

Note to reviewers

Summary by CodeRabbit

New Features

Updated the webpage title for improved readability and branding consistency.

is this your First Contribution ?


[vcpkg_replace_string] warn unchanged by call

If a call to vcpkg_replace_string makes no changes i.e doesn't effectively replace a string, A warning is logged.
This should also help identify ports that no longer need these calls to fix things in .pc files etc.

I like the warning. (And the idea is not new ;-)
But I wonder if there should also be an OPTIONAL keyword added, for replacements which are needed only in some contexts.
(And once we start to add options, I would also add REGEX to that list.)
(And once we start to touch this function, I would like to forward the observation that the function is changing the line endings which may break other stuff, cf. https://github.com/microsoft/vcpkg/pull/34693#discussion_r1370588016.)
Don't update the PR too quickly during discussion. CI will rebuild many ports. And this is the reason why I would prefer to have a small discussion about what needs to go into a single merge before forcing rebuilds on users.

I like the warning. (And the idea is not new ;-)

I figured, I was just frustrated that I couldn't easily confirm that one of these calls was doing absolutely nothing. :stuck_out_tongue_winking_eye:

But I wonder if there should also be an OPTIONAL keyword added, for replacements that are needed only in some contexts.

Personally, I would prefer
if (context)
    vcpkg_replace_string(${filename} ${match} ${replace})
endif ()

over
vcpkg_replace_string(${filename} ${match} ${replace} IGNORE_UNCHANGED)

Because it's too easy to do the wrong thing in the latter, and In a PR how do you tell if it is the wrong thing? you'll probably ask for context anyway.

(And once we start to add options, I would also add REGEX to that list.)

hmmmm, I guess that could be as simple as something like
function(vcpkg_replace_string filename match replace)
    if (ARGV3)
        set(regex "REGEX")
    else ()
        set(regex "")
    endif ()
    file(READ "${filename}" contents)
    string(SHA512 before_hash "${contents}")
    string(${regex} REPLACE "${match}" "${replace}" contents "${contents}")
    string(SHA512 after_hash "${contents}")
    if (before_hash STREQUAL after_hash)
        message(WARNING "vcpkg_replace_string(\"${filename}\" \"${match}\" \"${replace}\") made no changes.")
    endif ()
    file(WRITE "${filename}" "${contents}")
endfunction()

but I'm not sure how much time I have to put into this.

(And once we start to touch this function, I would like to forward the observation that the function is changing the line endings which may break other stuff, cf. #34693 (comment).)

This sounds like a bug in CMake to me, I wonder if there in any word there on this.

(And once we start to touch this function, I would like to forward the observation that the function is changing the line endings which may break other stuff, cf. #34693 (comment).)

This sounds like a bug in CMake to me, I wonder if there in any word there on this.

https://gitlab.kitware.com/cmake/cmake/-/issues/23614

Personally, I would prefer
if (context)
    vcpkg_replace_string(${filename} ${match} ${replace})
endif ()

over
vcpkg_replace_string(${filename} ${match} ${replace} IGNORE_UNCHANGED)

Because it's too easy to do the wrong thing in the latter, and In a PR how do you tell if it is the wrong thing? you'll probably ask for context anyway.

In general, I agree. OTOH there are cases where we know all possible inputs, but the condition is complex (e.g. target system plus linkage except for toolchain xxx.)

(And once we start to touch this function, I would like to forward the observation that the function is changing the line endings which may break other stuff, cf. #34693 (comment).)

This sounds like a bug in CMake to me, I wonder if there in any word there on this.

https://gitlab.kitware.com/cmake/cmake/-/issues/23614

It doesn't sound like this can be reasonably worked around for all cases.
From what I can tell, you'd have to use either file(CONFIGURE) or file(GENERATE) and both will attempt to resolve variables of some form, so you'd have to work around those just to work around the EOL conversion. :stuck_out_tongue_winking_eye:

Personally, I would prefer
if (context)
    vcpkg_replace_string(${filename} ${match} ${replace})
endif ()

over
vcpkg_replace_string(${filename} ${match} ${replace} IGNORE_UNCHANGED)

Because it's too easy to do the wrong thing in the latter, and In a PR how do you tell if it is the wrong thing? you'll probably ask for context anyway.

In general, I agree. OTOH there are cases where we know all possible inputs, but the condition is complex (e.g. target system plus linkage except for toolchain xxx.)

After writing my response below, I think this is actually the wrong discussion to be having. I think the real questions are, who is this for? and why do they want it? If it's for the reviewers so that they can request changes to avoid unnecessary or potentially harmful code, then you probably don't want it optional. Or is it for devs to quickly hack together a port and not care about edge cases, in which case is it worth keeping just be handy to some? Or yes but needs to be bundled with regex support?

If it gets that specific, it sounds like you're making some pretty broad assumptions about it being harmless in every other combination. And even if you don't assume and do the tests manually, is that going to be done by the next person who updates the version to make sure all cases are still harmless?
But, I guess the context could be specified within the match
vcpkg_replace_string(${filename} "(${target}-${linkage}) to-change" "\1 changed" REGEX IGNORE_UNCHANGED)

however, even if it's possible to do a negative lookahead/lookbehind with the minimal regex CMake provides, I'd argue it'd be much slower to parse (by humans) than something like if (NOT TOOLCHAIN STREQUAL "xxx").

"Broad assumptions"... "reviewers" ... "next person"...
Actually I ask for OPTIONAL because my experience with contributions and reviews shows that platforms which are not tested in CI are unsufficiently considered by the "next person" who modifies a port and even by reviewers. And too often, warnings trigger quite uninformed suggestions from reviewers, unfortunately.
And there is no link between OPTIONAL and REGEX. The latter is for cases where the function can't be used at all at the moment.
I would like to check CI output, but the first run didn't finish before pushing the new changes, so the output is lost.

Actually I ask for OPTIONAL because my experience with contributions and reviews shows that platforms which are not tested in CI are unsufficiently considered by the "next person" who modifies a port and even by reviewers. And too often, warnings trigger quite uninformed suggestions from reviewers, unfortunately.

That sounds like a good enough reason to me, I hope the most recent changes are adequate.

And there is no link between OPTIONAL and REGEX. The latter is for cases where the function can't be used at all at the moment.

agreed, I phrased that poorly, I was just reiterating your initial point to me.

I would like to check CI output, but the first run didn't finish before pushing the new changes, so the output is lost.

sorry, I thought I saw somewhere that [ci skip] worked on GitHub and/or you'd have to sleep at some point :stuck_out_tongue_winking_eye:
So I figured I should just update when I had time, in case I don't later.

@gerard-ryan-immersaview, Is work still being done for this PR?

@gerard-ryan-immersaview, Is work still being done for this PR?

It looks like I was awaiting feedback and/or the pipelines to complete but those logs have expired now.

apologies for the spam, it looked like the CI was generating a few more occurrences after the rebase so I'll let this pipeline complete without any fixes to gather a more complete list of errors before fixing them.

@BillyONeal I pushed some fixes for the CI but feel free to cancel this if needed.
These fixes aren't super relevant just yet.

@gerard-ryan-immersaview Why did you revert the merge with master I pushed to unblock this?

@gerard-ryan-immersaview Why did you revert the merge with master I pushed to unblock this?

hmm, I have a push mirror to this that probably did something, I deleted it for now.

Thanks, restored the merge!

mimalloc triggered the warning because match and replacement were the same; I disabled the warning in that case.

@JonLiu1993 I'm trying to make sure this one lands so please leave reviewed on it even if some failures come back

Hurray, finally! Thanks for the new warning!


[Feature] Add a onClick trigger for icon widget

Summary
The icon widget used inside a modal can have an onClick trigger to execute something when the modal is closed

@hetunandu Should this instead be modal property like onClose/onEscapeKeyDown/onBackdropClick? Additionally onRendered can also be useful.

@somangshu if i am not wrong, the modal does not really have control over when it is closed. You point is valid that one can close the modal without clicking on the icon too. I think having a listed icon widget with onclick still makes sense. We can probably divide these into two separate issues

@hetunandu check this out.
What you have suggested makes sense, One follow up question. For example as you suggested in the use case, The onClick of the icon will be used to close the model, How do I then add the event ex: afterClose?

Let's divide this up into two issues

Add a onClose event for a modal widget which will get triggered whenever for any way the modal is closed

Icon widget is not listed in the widget today. Lets add an icon widget with the onClick event for use cases where the icon click matters

The second requirement to get the icon button as a separate widget will be taken up in #4353.
In this issue, we will be resolving the onClose property in the  modal widget


Added property generation from ctor and fixed a bug where declaration was generated outside of class.

Awesome! Thanks ❤