Add a nice landscape image to README

@derekprior I'm not 100% clear on what should go in a gemspec's files list, but I made it only lib/ and a few doc files.

I think we should leave the gemspec as is and upload the image to s3/images.thoughtbot.com. The docs render the readme, and as is, the image is broken.

Woo! I got a commit in!


Version 2.1.1 worked fine on Xcode 8 but now fails!

My production build was working perfect with version 2.1.1 and Xcode 8 but
it started to throw up build error all of a sudden.
Digging deeper found the root-cause due to the plugin.xml of 2.1.1
points to https://github.com/mapsplugin/cordova-plugin-googlemaps-sdk.git
which demands Xcode 9 (see changenote)
This dependency should have been:
dependency id="com.googlemaps.ios" url="https://github.com/mapsplugin/cordova-plugin-googlemaps-sdk.git#2.6.0"
It seem to be taken care in 2.3.1 - is it possible to update other releases as well.

You need to update Xcode 10. No longer support xcode8


Fix rewrite_tag match rule and trim start of string pattern

What this PR does / why we need it:
Which issue(s) this PR fixes:

Fixes #689
Does this PR introduced a user-facing change?

Additional documentation, usage docs, etc.:

I think this can be backported to release-2.2 to make the fix available in 2.2.1

I think the fix can also be backported to release-2.2 to make the fix available in 2.2.1

Agree

It might not be clear but the fix is to match with a tag prefix instead of entire tag in rewrite_tag config. When a prefix does not exist (i.e.  the tag is non-expansive), do not generate a rewrite tag config since it will not work based on https://docs.fluentbit.io/manual/installation/kubernetes since the cached k8s metadata is absent from the tag.


Update VS2015 project to set module variable definitions

Some additional fixes for VS compilation as well.

This and #527 will address the assert logged in #514

@plusvic, this should also take care of the basic Windows CI #523 as well. AppVeyor supports nicer GitHub integration (like Travis) but that requires tokens and things that I didn't want to bother with. This will simply add a comment to the PR thread indicating if the build passed or failed.
All you should have to do is create the yara project in AppVeyor. It will pick up the build config from the yml file.
From the documentataion:

Project builds can be configured by either appveyor.yml or on the user interface. ...

If you have appveyor.yml in your repo it will override all settings made on the UI unless explicitly disabled by Ignore appveyor.yml. The only exceptions are environment variables and notification settings. Environment variables defined on UI are getting merged with those ones defined in appveyor.yml. Variable values with the same names are getting overridden with values from UI.

Already addressed in other commits.


[SortedCollections] Disable tests with @testable imports in release builds

Checklist

[X] I've read the Contribution Guidelines
[X] My contributions are licensed under the Swift license.
[ ] I've followed the coding style of the rest of the project.
[ ] I've added tests covering all new code paths my change adds to the project (if appropriate).
[ ] I've added benchmarks covering new functionality (if appropriate).
[ ] I've verified that my change does not break any existing tests or introduce unexplained benchmark regressions.
[ ] I've updated the documentation if necessary.

@swift-ci test


Majlinda Lako - MRC scCornea.

Continued from https://github.com/HumanCellAtlas/hca-data-wrangling/issues/232
Primary Wrangler:
Ami
Secondary Wrangler:
Enrique

Associated Markdown
Found here
Google sheet:
https://docs.google.com/spreadsheets/d/11CLPU-rZrK7cH5YgepKJasm5UMNO32y8Zx2UuHehJc8/edit#gid=253173527
Latest Google Sheet Version:
https://docs.google.com/spreadsheets/d/1Gi-HT8_D-TYIHN4C4kRUW7_HtMkcMRHLNBrsA4LQrGM/edit#gid=65395503
Publication:
https://www.sciencedirect.com/science/article/pii/S1542012421000215?via%3Dihub
Key Events

[x] Contact them about their project for the MRC grant.
[x] Have a call/meeting with the team to assess project whereabouts.
[x] Send initial instructions about contributing, including questionnaire and T&Cs
[x] Receive project questionnaire and move project from potential to in_progress projects
[x] Assign the Primary Wrangler
[x] Receive T&C form filled out by at least 1 representative of group/project
[x] Send custom metadata template spreadsheet, how-to guide and directions for submitting data files
[ ] Receive spreadsheet (to a sufficient standard to submit dataset)
[ ] Receive data (to a sufficient standard to submit dataset)
[ ] Curate metadata spreadsheet with ontologies
[ ] Upload sheet to validate metadata (most likely in staging)
[ ] Check linking
[ ] Validate data files
[ ] Ask the Secondary Wrangler for an end-to-end review of the project. Ask the Expertise Wrangler to review specific tabs if needed
[ ] Get final approval of submission including metadata spreadsheet and data files
[ ] Submit dataset to Production and hand off tracking to data dashboard

Please track the below as well as the key events:

Track date first spreadsheet received and final spreadsheet sent by editing ticket to include
date next to event.
Track spreadsheet iterations by placing asterisks next to receive spreadsheet event.
Track any metadata issues/tickets made for dataset with a bulleted list of links under
received spreadsheet event. Links should be to the ticket in the metadata repo.

24/06/2019: Sent e-mail to plan catch-up call
25/06/2019: Set up call for July 4th, 2019
24/06/2020: Sent spreadsheet and instructions for filling the spreadsheet and start data upload

Received an email on July 22, due to time constraints they are going to submit to SRA but will come back to us before the end of the year to submit to DCP.

BioRxiv manuscript here: https://www.biorxiv.org/content/10.1101/2<IP_ADDRESS>438v1

There is not enough metadata to begin wrangling this dataset. Once the manuscript is ready or they come back to us, I will begin working on this. For now I'll put it in the icebox

There is a recently published HCA paper that looks related to the pre-print above:
Collin J, Queen R, Zerti D, et al. A single cell atlas of human cornea that defines its development, limbal progenitor cells and their interactions with the immune cells. The Ocular Surface. 2021 Apr. DOI: 10.1016/j.jtos.2021.03.010
PMID: 33865984

Data availability: GSE155683

This dataset contains ATAC-Seq, which we can't push to the DCP until we get the Read3 enum value

Wrangling this dataset.

I have reviewed the spreadsheet! here are my comments:
Project

Please add the cell count estimate (21343)
More than a review, remember to add the UCSC cell browser link (http://retinalstemcellresearch.co.uk/CorneaCellAtlas/) into the external portals spreadsheets so that it can be properly linked in the data portal

Project - Contributors

Linda lako’s email is wrong. Should be<EMAIL_ADDRESS>
Donor organism

Missing HDBR accessions for the developmental donors (It’s the ID that you used)
I’d change the “fetal stage” for more specific ontologies (e.g. “10th week post fertilisation human stage”)
All non-diseased donors should have “normal” in the disease status

Specimen from organism

the amniotic membrane specimen does not look like a specimen used in the experiment for me. It looks like they obtained the amniotic membrane from other sources, and used it as a growth medium for the cell lines. I’d take it out of specimens
In the specimen with limbal dysplasia, I am not entirely sure about the term you have chosen. Doing some searching I have found “limbal stem cell deficiency”, which seems to be more accurate, although I have no idea if this is the correct term either. There’s a bit more info here https://eyewiki.aao.org/Limbal_Stem_Cell_Deficiency

Cell line

Continuing with the amniotic sample, I’d just remove the linked specimen

Sequence file

It’s missing all the sequence file filenames and some metadata flagged in yellow
Missing process ID for grouping

Analysis protocol

Should have “Alignment”, “cell filtering”, “gene filtering”, “doublet removal" in the single cell RNA seq derivation process. I would also remove from the description the second part (referring to the creation of the uMap/tSNE)

Analysis files

I’d add the files that you can download from the UCSC cell browser (http://retinalstemcellresearch.co.uk/CorneaCellAtlas/), at least the ones related to gene expression and cell type annotation

Please address these comments and let me know when the spreadsheet is ready for another review :)

Requested the fastq files from NCBI cloud delivery service. Only 1 fastq file is available via ENA. Hopefully NCBI will provide the paired-end reads.

Thanks @ESapenaVentura , I have made all your suggested changes except:

The ontology term as we discussed, and I have commented on what we agreed for now above.

Sequence file names and scATAC-seq reads metadata: I will fill these once I receive the fastq from NCBI cloud delivery

I added the cell annotations files from the linked portal (http://retinalstemcellresearch.co.uk/CorneaCellAtlas/). However the gene expression files were large so I didn't download and add those. The counts files are already available, downloaded and names added to the google sheet via GEO.

Here is the updated version if you could re-review to double-check all ok, thanks!
https://docs.google.com/spreadsheets/d/1Gi-HT8_D-TYIHN4C4kRUW7_HtMkcMRHLNBrsA4LQrGM/edit#gid=65395503

Next ontology term request: https://github.com/HumanCellAtlas/ontology/issues/100

upload area: 75b63f7f-1eb8-473c-b751-8b9376563c22

Submitted.

Assigned E-HCAD id: E-HCAD-43

Re-assigning this dataset to E-HCAD-50.

Manually curated the idf and sdrf files and uploaded them to Gitlab: https://gitlab.ebi.ac.uk/ebi-gene-expression/scxa-metadata/-/merge_requests/294
E-HCAD-50

This dataset needs updating. The library prep. method should be 10X 3' v3 not Drop-seq. I edited it in ingest and now it is raising an error because no spatial barcode info is added. However this is not a 10X visium dataset: https://contribute.data.humancellatlas.org/submissions/detail?uuid=50d64edf-6fbe-48ea-89aa-80f3543dd077&project=6ac8e777-f9a0-4288-b5b0-446e8eba3078

Submitted update.

Submitted updates.

exported.

looks as expected in the hca data portal.


Target dependencies are broken for "make all_unity"?

Hi,
I tried to apply the latest Cotire for my project, but the target dependencies are completely broken if I try to use unity build. My targets are in different subdirectories and the dependencies are specified with add_dependencies(). I execute: "make all_unity". I cotired almost all of my targets.

Let me try to add a dependency getter to CMake 3.7. I have also a test case available at gjasny/cotire@4f1b89893cd79a5abd95af1b539b87ab081a2e03
CC: @grover

I wrote a PoC getter for the utility dependencies in CMake: gjasny/cmake@f85065ad781918e3ebf3dd336dcec8fe6e3f4cac
You can retrieve the deps via get_target_property(_deps example DEPENDENCIES). Where would you add that information in cotire.cmake?

CMake issue: https://gitlab.kitware.com/cmake/cmake/issues/16165

This just got merged into CMake master: https://gitlab.kitware.com/cmake/cmake/merge_requests/407

The MANUALLY_ADDED_DEPENDENCIES property has been released with CMake 3.8.0

Just had to work around this.   I'd love to see this supported.


Expose backend as an onion service

Wasabii backend was exposed as an onion service until we lose the servers. We have to restore that because there are users who had they wallet configured to access to that endoint.
We can take the opportunity to implement this to be automatically available by default instead of having to manually configure Tor.

Done


Cosmos getTransactionData

Map TxResponse in cosmos sdk client properly
function  txsHashGet(hash: string): Promise
line 231 client.ts

I'm working on it 💪 🔥

closing, issue was fixed in pr 691


Revert xterm addon license workaround

Revert https://github.com/microsoft/vscode/commit/88f10ca5ed57020a92e09c5548bfd2d2cca96425 once https://github.com/xtermjs/xterm.js/issues/2257 is done.

I don't think I need to do anything here.


[iOS] Add AirPlay 2 Support

Hi there guys,
I was using react-native-track-player, and found its Airplay support was already mostly there! Yet, I ran into two hiccups:

When setting AirPlay output, the audio volume is disabled on the device
The Apple TV to which I output shows it as if it's a video stream, with metadata missing

I consulted the internet, and these two problems seem relatively simple to fix (see here for instance), by setting allowsExternalPlayback to false on AVPlayer. This (counter-intuitively) sets the AirPlay stream to audio, rather than video, opening up volume controls and settings the proper metadata when streaming to the Apple TV.
Additionally, I made an effort to set up AirPlay 2 properly for this application. Fortunately, most of the work is already done, with the docs only recommending to set a RouteSharingPolicy, which I have implemented. Unfortunately, I have no means if AirPlay 2 is actually working, but I these settings do no harm regardless.
This pull request at the very least improves QOL for those who wish to use react-native-track-player in combination with AirPlay, and I would recommend it be merged.
I am here for any additional questions.
Cheers,
Lei

Before  | After
 |

Thanks for your PR! What would we need to do to test Airplay 2?

@puckey I am still trying to find a programmatic way to do this. However, you could also try and stream to multiple speakers simultaneously. If this works, it proves Airplay 2 is used. I only have a single Airplay 2-capable device though 😅

Okay, in that case Airplay 2 support is already there – I have multiple Homepod / Sonos speakers and have tested RNTP on them.. But that does not take away that the way you have set it up is more correct. I will test this on an Apple TV and merge if everything works well.

Perfect, thank you!

Are we good to merge? I'd love to switch back to this package for my projects 😅

May we have this merged?

May we have this merged?

@puckey did you ever get a chance to check this?

Released in 1.2.4


Missing login_hint param in loginUrl

https://github.com/ringcentral/ringcentral-js/blob/4.x/sdk/src/platform/Platform.ts
loginUrl method should also take loginHint as an argument and set the login_hint query parameter in the constructed url.
login_hint is a standard optional url parameter for oAuth2 authentication.
https://auth0.com/docs/authenticate/identity-providers/pass-parameters-to-idps
The value passed pre-populates the email field in the RingCentral login form.

The changes have been made PR: https://github.com/ringcentral/ringcentral-js/pull/262, I'm closing this issue now feel free to reopen it if you have any doubts.


fix(SlideToggle): Refactor component

Graduation of the component from SEEK one to the version that we customized for SEEK Asia
Spec for SEEK Asia version:

I've also added back aria-checked and tabindex="0" attributes for better accessibility

Checking the preview, looks like it doesn't work when the label is on the left

Checking the preview, looks like it doesn't work when the label is on the left

The old tactics is pop and push label before or after the handle, but I let css do their job solely.


How to get s3 object url after uploading?

I went through the S3TransferManager-Sample demo. It works well. But my question is how to get the s3 object url (the uploaded file url)? Thanks

By the way, how to show the progress. I follow the demo, it seems I only got one callback. I don't know it's because of the size of the file is too small.

Hi bagusflyer,
For first question, you can use AWSS3PreSignedURL to generated a pre-signed downloadURL that can be used to download a file in S3 Bucket.  For usage details, please check out the sample app: S3BackgroundTransfer-Sample.
For second question,  AWSS3TransferManagerUploadRequest and AWSS3TransferManagerDownloadRequest supports uploading/downloading progress through  blocks:
 AWSS3TransferManagerUploadRequest *uploadRequest = [AWSS3TransferManagerUploadRequest new];
uploadRequest.uploadProgress = ^(int64_t bytesSent, int64_t totalBytesSent, int64_t totalBytesExpectedToSend) {
};
...

For more details, You can check out UploadViewController.m and DownloadViewController.m in S3TransferManagerSample app.


Nested scripts issue new to 4.1.0-beta2

Describe the bug
Prior to version 4.1.0-beta2, a script I used to check if the player had triggered a custom encounter before or not worked, but stopped working from this version.
The script included two scripts within it to check flags (as I use flags often, I'd made custom scripts to repeat the code less), one script to add a flag to a variable based on a value passed to the custom script, one script to check a variable has a flag again based on a value passed to the custom script.
I've attached a blank project with this happening.
To Reproduce
Steps to reproduce the behavior:

Go to Scene 1 and build
Run to your right to the end of the screen, on leaving the trigger it will let you know a flag has been added with 'Added Flag 4'.
Leaving the trigger a second time should let you know the flag has already been added with 'Flag 4 already set' if it's working, or it'll say the flag has been added again 'Added Flag 4'. if it hasn't worked.
To get it to work, disable the custom script 'Have I triggered this before' and enable the disabled 'Less nested event works, below doesn't' script group. This will work as intended, but break once nested another level.

Expected behavior
Previously, the script with two nested events worked and set the flag.
Screenshots

Platform (please complete the following information):

OS: MacOS 14.5
GB Studio version 4.1.0-beta2

Additional context
Attached sample project.
Debug Nested Includes.zip

Hi @joeledge Thanks for this, using this example project I believe I've managed to get a fix in place.
Your example project actually did have a small bug inside the "Have I triggered this before?" script

should have been this:

but even with this fixed there was still the problem.
The fixed build will be available here: https://github.com/chrismaltby/gb-studio/actions/runs/10582432095
I can confirm that trying your project with the previous build I saw encounters triggering multiple times but with this latest build they only trigger once.

Excellent, yes I've just tried that with the latest build and it's fixed it (sorry about the rather too quickly put together debug project!), thank you!


Standardised output field name in perimeter-area ratio tool

This commit was done for two reasons:
1] Test I am doing updates to source code correctly - please verify John! I don't want to mess it up!
2] The output field name was originally P-A_RATIO. Hyphens are a bit of a no-no for databases and so I have simply adjusted this to P_A_RATIO. Seems trivial but this is inline with ESRI standards as discussed here: https://desktop.arcgis.com/en/arcmap/latest/manage-data/tables/fundamentals-of-adding-and-deleting-fields.htm#GUID-8E190093-8F8F-4132-AF4F-B0C9220F76B3. Yes shapefiles allow it but this will save users a whole world of pain when they try to import it into other applications.

This looks good to me.


#970 | Dheeraj | Add /on-fetch-modes API just to log the response

 Here is an overview of what got changed by this pull request:

Issues
======
- Added 1

Complexity increasing per file
==============================
- src/In.ProjectEKA.HipService/Link/Model/AuthModeFetch.cs  1
- src/In.ProjectEKA.HipService/Link/Model/OnFetchAuthModeRequest.cs  1

See the complete overview on Codacy


Use latest version of scala-libs > 14.0 - changes VHS models

This will require a migration of data held in the storage manifest VHS dynamo table.

Gonna retract my review for now as I think there needs to be much more changes for this to work?

Quite possibly - this is me getting in a draft PR early (and getting travis to run the tests while i'm travelling).

:+1: no probs


microsofts azure Extensions issue

Repro steps:

Action: cosmosDB.executeAllMongoCommands
Error type: Error
Error Message: db.friends.find at 13:1: Error near line 17, column 0: 'mismatched input 'mongo' expecting '. Please check syntax.
Version: 0.12.2
OS: win32
Product: Visual Studio Code
Product Version: 1.43.2
Language: en

Call Stack
extension.bundle.js:245:242732extension.bundle.js:245:242732
u extension.bundle.js:245:239243
processTicksAndRejections task_queues.js:85:5

@prathap-web can you give us the exact command that was giving you this error


Add multiple suites for linux platform

usecase-wrt-linux-tests

Add cases for Update without remove old data
Add cases for Open external URL (dynamic/static)
Add cases for flash plugin load and display
Add cases for csp

wrt-apptools-linux-tests

Add pyunit cases for create,pkgName,build

wrt-webappmanu-linux-tests

Add web sample apps and hosted apps

@yinhaichao Please update wrt-webappmanu-linux-tests ==> webappmanu-linux-tests in crosswalk-test-suite/tree/master/misc folder.

Please add unit test results for
usecase-wrt-linux-tests
wrt-apptools-linux-tests

There are 1036 changed files from more than one test suite in on PR, it's really too difficult to review. Better to separate them into one PR one test suite.
And please move wrt/wrt-apptools-linux-tests to apptools/apptools-linux-tests

I will separate test suites into separate PRs, so close it


Include better failure information in swift-test xunit output

Description
Problem
Currently, when creating an xunit xml output file with the --xunit-output option in swift test, test failures generate an empty <failure> tag:
 <testcase classname="ExampleTests.Tests" name="testFail" time="0.041534993">
 <failure message="failed"></failure>
 </testcase>

This doesn't really provide much useful information for anything that may want to consume this information (CI being the prime example). The xunit xml spec allows any string to be inside this tag.
I am not intimately familiar with XCTest, so as the fixme mentioned below indicates it may be that there just needs to be better reporting given by XCTIssue to include more relevant details.
Possible Fix
As the FIXME comment here suggests, we could simply stick the result.output onto line 1284
"""
            if !result.success {
                content += "<failure message=\"failed\">\(result.output)</failure>\n"
            }
"""

which generates XML that looks like this (just has stdout from the test invocation mushed into it):
<testcase classname="ExampleTests.Tests" name="testFail" time="0.029471092">
<failure message="failed">Test Suite 'Selected tests' started at 2024-06-01 21:02:59.244
Test Suite 'ExampleTests' started at 2024-06-01 21:02:59.246
Test Case 'Tests.testFail' started at 2024-06-01 21:02:59.246
/home/test-ex/Tests/test.swift:8: error: Tests.testFail : XCTAssertEqual failed: ("1") is not equal to ("2") -
/home/test-ex/Tests/test.swift:9: error: Tests.testFail : XCTAssertEqual failed: ("foo") is not equal to ("bar") -
/home/test-ex/Tests/test.swift:10: error: Tests.testFail : XCTAssertTrue failed -
Test Case 'Tests.testFail' failed (0.001 seconds)
Test Suite 'ExampleTests' failed at 2024-06-01 21:02:59.247
     Executed 1 test, with 3 failures (0 unexpected) in 0.001 (0.001) seconds
Test Suite 'Selected tests' failed at 2024-06-01 21:02:59.247
     Executed 1 test, with 3 failures (0 unexpected) in 0.001 (0.001) seconds
<failure>

That isn't ideal, but it's at least more detail than is given currently
If the full output is too verbose, there's plenty of room to process it down to something cleaner, but that would obviously be beholden to the output format remaining the same.
This does seem generally like it may need a wider refactor to pull additional information from the test reporter though, adjacent issues:

https://github.com/apple/swift-package-manager/issues/4752
https://github.com/apple/swift-package-manager/issues/7383

Expected behavior
xunit.xml failures include relevant failure information
Actual behavior
xunit.xml failures are empty
Steps to reproduce
Run swift test --parallel --xunit-output test.xml in any project with a test failure
Swift Package Manager version/commit hash
5.10
Swift & OS version (output of swift --version && uname -a)
Swift version 5.10 (swift-5.10-RELEASE)
Ubuntu 20.04 & macOS

Note that the output schema more closely matches JUnit.

Currently, the XCTest implementation of xUnit (JUnit) XML output is based solely on the exit status of the process hosting each test. Additional information such as the cause of a failure is not available. Resolving this problem will require an overall refactoring of how XCTest produces its XML output. That work is tracked by #4752, so I'm going to mark this issue as a duplicate of that one.

Duplicate of #4752


Add platform Ezviz

Description:
Ezviz platform
Related issue (if applicable): fixes #
Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#11596
Example entry for configuration.yaml (if applicable):
# Example configuration.yaml entry
camera:
  - platform: ezviz
    username<EMAIL_ADDRESS>    password: 'your_password_here'
    cameras:
      D12345678:
        username: admin
        password: ABCDEF
      D87654321:
        username: admin
        password: ABCDEF

Checklist:

[x] The code change is tested and works locally.
[x] Local tests pass with tox. Your PR cannot be merged unless tests pass
[x] There is no commented out code in this PR.
[x] I have followed the development checklist

If user exposed functionality or configuration variables are added/changed:

[ x] Documentation added/updated in home-assistant.io

If the code communicates with devices, web services, or third-party tools:

[x] The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
[x] New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
[x] Untested files have been added to .coveragerc.

If the code does not interact with devices:

[ ] Tests have been added to verify that the new code works.

Hi,
I'm trying to implement PTZ, but I face an issue:
        params = [direction, speed]
        ret, _ = await self.hass.async_add_executor_job(
            self._ezviz_camera.move, params
        )

        if ret != 0:
            _LOGGER.error("Error movingcamera %s': %s", self._name, ret)
            return

params are not passed to self._ezviz_camera.move when it's launched. I tried to pass "*params" and it does not work either. Any idea ?
Thx a lot!

I introduced the support of switches:

IR led
presence led
privacy mode
etc/

What is the best way to integrate it within the platform ? Via switches or directly via a service on the camera ?
Thanks for your help
Best
Pierre

I introduced the support of switches:

We want to keep the scope as small as possible for the initial PR.  Therefore, please do not add any additional functionality and remove the custom services recently added.  These are all items to be added to subsequent PRs once we get the first one through.
Let me know when this is ready (meaning you're not adding new commits) and I'll review.

Ah ok, I was hoping to keep going on the dev as there were a lot of features missing.
So, I can re-push a previous commit maybe ?

Ah ok, I was hoping to keep going on the dev as there were a lot of features missing.
So, I can re-push a previous commit maybe ?

You can certainly do that if an older commit represents what you believe is done for the minimal functionality.    It's your decision.  :smile:

Here is the new PR:
https://github.com/home-assistant/home-assistant/pull/30763
(pity that we can't integrate the final version...)

Here is the new PR:
#30763
(pity that we can't integrate the final version...)

We do not permit PRs to be opened that are dependent on other unmerged PRs and I have closed the other PR meanwhile.  We can reopen it once this is done.
The reason we ask the initial PR be kept small (and not submit dependent PRs) is that there is a high likelihood you will need to make more changes to this PR before it can be merged.  You can see more about these rules here: https://developers.home-assistant.io/docs/en/creating_component_code_review.html#5-make-your-pull-request-as-small-as-possible

Ok I understand the rule. But I thought we could close this particular one (30378) and keep the new one.
Any tip to revert 30378 to a previous commit ? ( "regenerated requirements" 848b317 )

But I thought we could close this particular one (30378) and keep the new one.

When you do that, we lose all the review history and changes, so it's not ideal.  If you decide to do that, make sure this one is closed first and then referenced as in the new PR that is replacing it...

Up?

It seems none of Andrew's comments have been addressed. Did you forget to push your changes after marking the comments as resolved?
I've unresolved the comments for now.

I my have messed up some stuff while reverting to the initial commit.
I'll try to fix all of this during the week-end. Thx


bug(docs): update readme, but using feat for a patch bump

:tada: This PR is included in version 2.0.1 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


PROMs Patient UI improvements: CARDS-1747, CARDS-1748, CARDS-1749, CARDS-1750, CARDS-1751

Included in this PR:

CARDS-1747: In longer forms that require scrolling, the radiobuttons that are right under the header cannot be clicked
CARDS-1748: Unnecessary gap between the form and the [Continue to...] button
CARDS-1749: While waiting to load the submitted answers for review, the [Update this survey] button is displayed on the side of the form title, only to move under once the contents are loaded
CARDS-1750: [Submit my answers] is the only main action button that is not centered and that is sticky to the bottom of the screen
CARDS-1751: Misplaced circular progress when clicking on [Continue to <next questionnaire>]

Testing:

Follow the JIRA task description for details
CARDS-1748 may impact forms in admin/clinician UI; make sure there are no unwanted side effects
CARDS-1751 may impact main action buttons in admin/clinican UI (e.g. the (+) buttons on Dashboard, Subjects, Forms, admin/Questionnaires, etc, as well as the form's Save button); make sure there are no unwanted side effects

Reviewing:

There is a separate small commit for each task. Reviewing commit by commit rather than file by file is recommended.

RE CARDS-1749 : Minor improvement (if possible) would be to do not show UPDATE THIS SURVEY buttons until the contents are loaded.

Because of how the review screen is rendered at this time, this is not trivial to do without refactoring. We should merge this as is and do the improvement later.

RE CARDS-1749 : Minor improvement (if possible) would be to do not show UPDATE THIS SURVEY buttons until the contents are loaded.

Because of how the review screen is rendered at this time, this is not trivial to do without refactoring. We should merge this as is and do the improvement later.

PR #1025 addresses this issue.


Add static code linters

I added a bunch of static code linters as well as an editorconfig file. It's all in separate commits, so we can have a pick. I would personally like to use them all, since they all make our lives easier.

Codecov Report

Merging #221 (27b2d0f) into master (adcd0aa) will increase coverage by 0.02%.
The diff coverage is 89.58%.

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   94.81%   94.83%   +0.02%
==========================================
  Files           7        7
  Lines         212      213       +1
==========================================
+ Hits          201      202       +1
  Misses         11       11

Impacted Files
Coverage Δ

hijack/signals.py
100.00% <ø> (ø)

hijack/helpers.py
94.62% <82.60%> (-0.06%)
:arrow_down:

hijack/templatetags/hijack_tags.py
88.09% <91.66%> (+0.59%)
:arrow_up:

hijack/apps.py
100.00% <100.00%> (ø)

hijack/checks.py
100.00% <100.00%> (ø)

hijack/middleware.py
93.33% <100.00%> (ø)

hijack/settings.py
100.00% <100.00%> (ø)

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 adcd0aa...27b2d0f. Read the comment docs.

Codecov Report

Merging #221 (27b2d0f) into master (adcd0aa) will increase coverage by 0.02%.
The diff coverage is 89.58%.

@@            Coverage Diff             @@
##           master     #221      +/-   ##
==========================================
+ Coverage   94.81%   94.83%   +0.02%
==========================================
  Files           7        7
  Lines         212      213       +1
==========================================
+ Hits          201      202       +1
  Misses         11       11

Impacted Files
Coverage Δ

hijack/signals.py
100.00% <ø> (ø)

hijack/helpers.py
94.62% <82.60%> (-0.06%)
:arrow_down:

hijack/templatetags/hijack_tags.py
88.09% <91.66%> (+0.59%)
:arrow_up:

hijack/apps.py
100.00% <100.00%> (ø)

hijack/checks.py
100.00% <100.00%> (ø)

hijack/middleware.py
93.33% <100.00%> (ø)

hijack/settings.py
100.00% <100.00%> (ø)

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 adcd0aa...27b2d0f. Read the comment docs.

It seems to me that we need to specify the versions of the linters we are using.

Hm… do you think so? I usually use the latest version, it's one more thing to update. I know things break sometime, but it's usually less work to fix it, than to keep up with updates. Especially since not all packages use PEP440 compatible semver versioning.
So, up to you, but I would be OK as is. 🤷

It seems to me that we need to specify the versions of the linters we are using.

Hm… do you think so? I usually use the latest version, it's one more thing to update. I know things break sometime, but it's usually less work to fix it, than to keep up with updates. Especially since not all packages use PEP440 compatible semver versioning.
So, up to you, but I would be OK as is. 🤷

Hm… do you think so? I usually use the latest version, it's one more thing to update. I know things break sometime, but it's usually less work to fix it, than to keep up with updates. Especially since not all packages use PEP440 compatible semver versioning.

The solution might be to add a simple requirements.txt file and connect to dependabot.

Hm… do you think so? I usually use the latest version, it's one more thing to update. I know things break sometime, but it's usually less work to fix it, than to keep up with updates. Especially since not all packages use PEP440 compatible semver versioning.

The solution might be to add a simple requirements.txt file and connect to dependabot.

@Mogost that was an excellent suggestion. Let me know if you have any further suggestions.

@Mogost I want to resolve this soon to rebase all other work on this. Any final remarks? I will make sure enable dependabot.


MassTransit Quartz integration breaks Quartz jobs

Is this a bug report?
Yes
Can you also reproduce the problem with the latest version?
Yes
Environment

Operating system: Windows 10
Visual Studio version: Rider 2020.3
Dotnet version: 5.0-rc1

Steps to Reproduce

Add Quartz with AspNetCore integration
Add MassTransit and configure message scheduler
Add any Quartz job with injected service (DbContext, Logger, eg)
Start host

Expected Behavior
Quartz jobs creating with MicrosoftDependencyInjectionScopedJobFactory, MassTransit jobs creating with MassTransitJobFactory
Actual Behavior
MassTransit overrides JobFactory and breaks all existing Quartz jobs
// SchedulerBusObserver.PostStart
_scheduler.JobFactory = new MassTransitJobFactory(bus);

Reproducible Demo
services.AddQuartz(options =>
{
        // Store configuration, etc

        var jobKey = new JobKey("RequestSentJob", "Template");
	options.AddJob<RequestSentJob>(job =>
	{
		job.WithIdentity(jobKey)
			.StoreDurably()
			.WithDescription("Periodical request sent");
	});

	var triggerKey = new TriggerKey("RequestSentJobTrigger", "Template");
	options.AddTrigger(trigger =>
	{
		trigger.ForJob(jobKey)
			.WithIdentity(triggerKey)
			.WithCronSchedule("*/3 * * * * ?*");
	});
}).AddQuartzServer(options => options.WaitForJobsToComplete = true);

services.AddMassTransit(options =>
{
	options.AddMessageScheduler(new Uri("queue:masstransit.quartz"));

	options.UsingRabbitMq((context, rabbitmq) =>
	{
                // Host configuration, etc

		rabbitmq.UseInMemoryScheduler(
			context.GetRequiredService<ISchedulerFactory>(),
			"masstransit.quartz");
        }
}

public class RequestSentJob : IJob
{
	private readonly IBus bus;
	private readonly ILogger<RequestSentJob> logger;

	public RequestSentJob(IBus bus, ILogger<RequestSentJob> logger)
	{
		this.bus = bus;
		this.logger = logger;
	}

	public async Task Execute(IJobExecutionContext context)
	{
		logger.LogInformation("Sending: RequestSent");

		await bus.Publish(new RequestSent
		{
			Text = DateTime.UtcNow.ToString(CultureInfo.InvariantCulture)
		});
	}
}

Thrown exception
Quartz.Core.ErrorLogger[0]
      An error occurred instantiating job to be executed. job= 'Template.RequestSentJob, message=Exception has been thrown by the target of an invocation.'
      Quartz.SchedulerException: Problem instantiating type 'AppAny.Template.RequestSentJob: Exception has been thrown by the target of an invocation.'
       ---> System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
       ---> Quartz.SchedulerException: The job class does not have a supported constructor: AppAny.Template.RequestSentJob
         at MassTransit.QuartzIntegration.MassTransitJobFactory`1.CreateConstructor()
         at MassTransit.QuartzIntegration.MassTransitJobFactory`1..ctor(IBus bus)
         --- End of inner exception stack trace ---
         at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
         at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
         at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
         at System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes)
         at System.Activator.CreateInstance(Type type, Object[] args)
         at MassTransit.QuartzIntegration.MassTransitJobFactory.CreateJobFactory(Type type)
         at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
         at MassTransit.QuartzIntegration.MassTransitJobFactory.NewJob(TriggerFiredBundle bundle, IScheduler scheduler)
         at Quartz.Core.JobRunShell.Initialize(QuartzScheduler sched, CancellationToken cancellationToken)
         --- End of inner exception stack trace --- [See nested exception: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an
invocation.
       ---> Quartz.SchedulerException: The job class does not have a supported constructor: AppAny.Template.RequestSentJob
         at MassTransit.QuartzIntegration.MassTransitJobFactory`1.CreateConstructor()
         at MassTransit.QuartzIntegration.MassTransitJobFactory`1..ctor(IBus bus)
         --- End of inner exception stack trace ---
         at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
         at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
         at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
         at System.Activator.CreateInstance(Type type, BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture, Object[] activationAttributes)
         at System.Activator.CreateInstance(Type type, Object[] args)
         at MassTransit.QuartzIntegration.MassTransitJobFactory.CreateJobFactory(Type type)
         at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
         at MassTransit.QuartzIntegration.MassTransitJobFactory.NewJob(TriggerFiredBundle bundle, IScheduler scheduler)
         at Quartz.Core.JobRunShell.Initialize(QuartzScheduler sched, CancellationToken cancellationToken)]

I found workaround
rabbitmq.UseInMemoryScheduler(
	// context.GetRequiredService<ISchedulerFactory>(),
	"masstransit.quartz");

But without configured scheduler Quartz using inmemory datastore =/

I think that MassTransitJobFactory can wrap existing JobFactory and fallback to it for non masstransit jobs

you can provide different scheduler factory just for MT one, there is an overload.

@NooNameR Which one?
I can use ISchedulerFactory.GetScheduler(string name), but i can't pass it in UseInMemoryScheduler
I think that scheduler = TaskUtil.Await(() => schedulerFactory.GetScheduler()); inside QuartzIntegrationExtensions.UseInMemoryScheduler can use overload with custom scheduler name and do not use default
I don't want to configure ISchedulerFactory outside DI, when i have one

to be honest, there is no right approach. MT could be use without DI, so job factory might be required...

ISchedulerFactory.GetScheduler() I suspect it should create new scheduler every time you called it. @lahma what do you think?

He saw the issue, and when he gets some time is planning to submit a PR.

ISchedulerFactory I think is called ONCE at startup.

Scheduler factory is a kind of a singleton, so it's a holder of configuration how to create a scheduler. Well actually you can build multiple schedulers by name but usually you have on scheduler per process so having multiple named ones isn't the most battle-tested use case. But for one service collection it only makes sense to have one such single factory as there's no support for named singletons.
So far my thinking is to adapt MassTransit's job factory to either work the same way as scoped/regular DI job factory that Quartz ships with (do service provider lookup) or even add possibility to hook some lifecycle events on Quartz's side when Quartz produces jobs. It's not a big deal to add service provider support which would allow jobs being injected with services in the container - or that's what I think so far.
What I've seen so far MassTransit wants to add some metadata to job data map and inject properties. The property injection on MT side probably is a bit more clever/faster and then there's the support for Uri type (haven't tested what Quartz feels about that OOTB).
But like Chris mentioned I probably will toy around with the possibilities and try to produce some kind of PR for discussion and refinement. This might involve opening some APIs or allowing variation in Quartz implementation. Would of course love to hear what bugs the most and hinders the ability to use polymorphism or similar tools.

@lahma I think if it possible to build separate scheduler only for MT purposes.
For example, pick your configuration from container/whatever, pass to MT, and MT will create separate scheduler for you. So everything will be encapsulated. I build kinda the same using Hangfire

I never in my entire life have used the Quartz instance that MassTransit owns for other job types. So I guess if I stipulate that as a constraint of using Quartz.NET with MassTransit – the problem is solved. Run a separate scheduler instance for your other stuff. You can prefix the Quartz tables with something else for your non-MassTransit jobs and still use the same database.

@phatboyg I have the same feeling, it doesn't make sense for me either.
I think there are two ways: either use Hangfire which supports it out of the box, or use Quartz as a separate service only for MT jobs

I might be on the other side of the spectrum when I think from resource perspective. Less databases and configuration I have to handle the better. Of course if it's a really busy instance then separate database might be warranted. You can run two schedulers against same database, just have different names for them and they won't interfere with each other (from logical perspective, resources are a different thing).

OK some rough edges but showing what I've played with. I've tweaked Quartz code a bit so this won't compile yet without Quartz release.
https://github.com/MassTransit/MassTransit/pull/2067

new job factory that inherits from Quartz's one, allow DI to jobs and simplified

https://github.com/MassTransit/Sample-Quartz/pull/1

configure Quartz and allow to use same scheduler as DI now understands the Microsoft DI being used
jobs and consumers can use DI
configuration with Options pattern, adding to existing, file being the starting point

Sorry for the noise on the second one, I was playing around and wanted to see recurring schedule in action with custom job.

I might be on the other side of the spectrum when I think from resource perspective. Less databases and configuration I have to handle the better. Of course if it's a really busy instance then separate database might be warranted. You can run two schedulers against same database, just have different names for them and they won't interfere with each other (from logical perspective, resources are a different thing).

I am using this approach now with overriden ShcedulerId and SchedulerNames, for now it is working fine(i guess), but i have to configure separate quartz SchedulerFactory

I put my own take on this, if it builds properly I'll merge it into develop for a package release.


⚠ Bofrost.de has degraded performance

In 1b34896, Bofrost.de (https://www.bofrost.de) experienced degraded performance:

HTTP code: 200
Response time: 1151 ms

Resolved: Bofrost.de performance has improved in 12a7087 after 12 minutes.


Re-adds git and golang packages

Re-adds the git and golang RPMs.  They are required for backplane install.
Signed-off-by: Christopher Collins<EMAIL_ADDRESS>

Maybe there should be some tests that check that in the backplane side of things?


Added click event support for the components.

Thanks for you contribution.
what is the use-case for clickable images / textblocks? Would it not be better to put them inside "clickable" elements - such as Button etc?

You're welcome.
Yes, you're right but I think it would be more useful that way.

This seems odd to me, it is more code to mantain to do the same thing that you can achieve nesting it in the clikable element... More code to mantain more bug, more problems

I analyzed a bit more and i think you are right, so you can close the pull request.

I looked up in the fontawesome documentation what it says about clickable icons and it is advised to be put into  or  tags etc. Therefor we should do the same, so that it seems more coherent with the official fontawesome lib.
Again, thanks for your pull request but I'll be closing it here.


[2Xpt] Recipe details. Team member #2

Should display a list of recipes created by the logged-in user as in the wireframe.
Should lead to recipe details.
If the user is the owner of the recipe, should allow the user to delete it.
Should have a modal to choose an inventory to create a shopping list with, as in the wireframe.

competed


Move the SIGNASSEMBLY precompiler directive so the setting can be tak…

…en off the command line, thus avoiding warning MSB3052.

Pull Request Test Coverage Report for Build 79984

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

Totals

Change from base Build 79980:
0.0%

Covered Lines:
4856

Relevant Lines:
6045

💛  - Coveralls

:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.3.1
:heavy_check_mark: No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.3.1


Back Album Art not saved correctly

Back album art seems to be saved as front album art

Fixed by https://github.com/NickvisionApps/Tagger/commit/7c7cf513e45dcfa2155df2b77e4041650f683202

Please try out the fix in this release: https://github.com/NickvisionApps/Tagger/releases/tag/2023.8.2-rc1


Publish confirmation modals should say name of correct Dataverse [OSF-4556]

Purpose
The modal dialogs used to confirm that a user wants to publish a dataverse or dataset used to always talk about the Harvard dataverse, rather than the dataverse provider specifically being used.
Changes
The host provider (for example, demo.dataverse.org) is found and this name is used in the modals instead of "Harvard".
After changes:

Side effects
No side effects. However, it should be noted that there are actually two modals that can be shown, and that without the fixes for OSF-4553 and OSF-6397, the second modal will never be able to be shown. (For when a dataverse is published but the specific dataset is not).
Ticket
https://openscience.atlassian.net/browse/OSF-4556

LGTM. Dropping this in RTM 💰


Add ArchiveAll, DeleteAll, and UnarchiveAll Commands

Fixes #70 and fixes #71
Previously, archiving, deleting, and unarchiving entries had to be done one at a time. Now you can archive/delete/unarchive all currently displayed entries in one go
Let's

add ArchiveAll, DeleteAll, and UnarchiveAll commands
change the error message when archiving an already archived entry (or unarchiving an already unarchived entry) to show the entry's name
fix some typos in the message usage of FindCompanyCommand and FindPersonCommand

Codecov Report

Merging #93 (13a59d0) into master (96e8ac9) will decrease coverage by 0.50%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master      #93      +/-   ##
============================================
- Coverage     44.77%   44.26%   -0.51%
  Complexity      463      463
============================================
  Files           112      115       +3
  Lines          2448     2476      +28
  Branches        357      360       +3
============================================
  Hits           1096     1096
- Misses         1266     1294      +28
  Partials         86       86

Impacted Files
Coverage Δ

...eedu/address/logic/commands/ArchiveAllCommand.java
0.00% <0.00%> (ø)

...ava/seedu/address/logic/commands/ClearCommand.java
100.00% <ø> (ø)

...seedu/address/logic/commands/DeleteAllCommand.java
0.00% <0.00%> (ø)

...edu/address/logic/commands/FindCompanyCommand.java
0.00% <ø> (ø)

...eedu/address/logic/commands/FindPersonCommand.java
100.00% <ø> (ø)

...du/address/logic/commands/UnarchiveAllCommand.java
0.00% <0.00%> (ø)

.../seedu/address/logic/parser/AddressBookParser.java
56.66% <0.00%> (-6.30%)
:arrow_down:

...rc/main/java/seedu/address/model/ModelManager.java
52.70% <0.00%> (ø)

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 96e8ac9...13a59d0. Read the comment docs.


Getting Gradle Error while running the App

Launching lib\main.dart on SM G610F in debug mode...
Running Gradle task 'assembleDebug'...
FAILURE: Build failed with an exception.

What went wrong:
java.util.concurrent.ExecutionException: org.gradle.api.GradleException: Failed to create Jar file C:\Users\Aniket.gradle\caches\jars-9\0cd2a3fddb44a9fb3cbb51e82ff8716a\gradle-1.0.0.jar.

org.gradle.api.GradleException: Failed to create Jar file C:\Users\Aniket.gradle\caches\jars-9\0cd2a3fddb44a9fb3cbb51e82ff8716a\gradle-1.0.0.jar.

Try:

Run with --stacktrace option to get the stack trace.
Run with --info or --debug option to get more log output.
Run with --scan to get full insights.

Get more help at https://help.gradle.org

BUILD FAILED in 29s
Error: Gradle task assembleDebug failed with exit code 1
To Reproduce
Steps to reproduce the behavior:

Go to the app directory
Enter flutter run
See error

Expected behavior
App should be run without an Error
Smartphone (please complete the following information):

Device: [SM G610F]

Hey There,
The issue that you have reported seems to be an error of the gradle on your system.
Have you tried and running a simple Helloworld Flutter app on the system on which you are getting this error ?
If you are able to run this hello world app and getting this error only in our code, then please do share the complete logcat output.

i am uble to run Hello world program, but in the bagisto, i am getting gradle version issues

Ok
Could you please do share the complete output of the flutter doctor command


Cargo update + License check

@grebnetiew, did a cargo update also validated the license mentions as they somehow deviated over time..

To do: test.
To do2: Consider replacing serde_yaml with serde_yml https://github.com/sebastienrousseau/serde_yml

Tested on petstore, still seems to work

@grebnetiew, I would suggest to look later into serde_yml. What do you think?

@grebnetiew, I would suggest to look later into serde_yml. What do you think?

I agree, since the serde_yaml crate was abandoned iirc. But this one seems a little immature in that it doesn't currently build correctly?
https://docs.rs/crate/serde_yml/latest

It seems like a good new issue for someone to look into and decide on and if positive, fix.


Bug with parallel assignment auto-correct

See #1831, the auto-correct function is broken with this edge case:
-        best_separator, best_score = separator, score if score > best_score
+        best_separator = separator
+        best_score = score if score > best_score

Both assignments should stay conditional.
As a side note, I don't get why this cop is in the Performance category - does it have any performance implications? Shouldn't it be in Style?

Shouldn't it be in Style?

It should be. I totally missed this.

Actually, I just came across https://github.com/JuanitoFatas/fast-ruby#parallel-assignment-vs-sequential-assignment-code - so I guess Performance makes sense too :smile:.

While this is true, the bigger value is the improved readability of the code, so I still think this should be a style cop.

//cc @rrosenblum

Agreed.

Thank you for the bug report. I agree, this is a bug. I will start working on a fix for it soon.
As for Performance vs Style, it falls into both categories. It was brought up as an old feature request and in talking about performance cops. When we are talking about the ability to assign variables in the millions per second, doubling the performance is negligible except in extreme cases. I agree that the bigger gain is readability, therefore making this better suited as a Style Cop. I will also go ahead and make this change.

Good stuff @rrosenblum :clap:


Dark mode fix (draft)

In this PR I'm going to modify the .svg files to add a white background so they can be seen in dark mode.

Docs Build status updates of commit a56563f:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

articles/media/1.svg
:white_check_mark:Succeeded
View

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

Docs Build status updates of commit 41fe5a8:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

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

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Docs Build status updates of commit 88651db:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

articles/concepts/circuits.md
:white_check_mark:Succeeded
View

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/2.svg
:white_check_mark:Succeeded
View

articles/media/3.svg
:white_check_mark:Succeeded
View

articles/media/4.svg
:white_check_mark:Succeeded
View

articles/media/5.svg
:white_check_mark:Succeeded
View

articles/media/6.svg
:white_check_mark:Succeeded
View

articles/media/7.svg
:white_check_mark:Succeeded
View

articles/media/8.svg
:white_check_mark:Succeeded
View

articles/media/8b.svg
:white_check_mark:Succeeded
View

articles/media/ItQPE.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

articles/media/beau.svg
:white_check_mark:Succeeded
View

articles/media/bloch.svg
:white_check_mark:Succeeded
View

articles/media/bloch2.svg
:white_check_mark:Succeeded
View

articles/media/concepts_tp.png
:white_check_mark:Succeeded

articles/media/draper.svg
:white_check_mark:Succeeded
View

articles/media/prelude_hadamardBloch.png
:white_check_mark:Succeeded
View

articles/media/stack.svg
:white_check_mark:Succeeded
View

articles/media/teleportation.svg
:white_check_mark:Succeeded
View

articles/media/tp.svg
:white_check_mark:Succeeded
View

articles/media/tp2.svg
:white_check_mark:Succeeded
View

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

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Docs Build status updates of commit aefd6ff:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

articles/concepts/circuits.md
:white_check_mark:Succeeded
View

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/2.svg
:white_check_mark:Succeeded
View

articles/media/3.svg
:white_check_mark:Succeeded
View

articles/media/4.svg
:white_check_mark:Succeeded
View

articles/media/5.svg
:white_check_mark:Succeeded
View

articles/media/6.svg
:white_check_mark:Succeeded
View

articles/media/7.svg
:white_check_mark:Succeeded
View

articles/media/8.svg
:white_check_mark:Succeeded
View

articles/media/8b.svg
:white_check_mark:Succeeded
View

articles/media/Concepts1.png
:white_check_mark:Succeeded

articles/media/ItQPE.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

articles/media/beau.svg
:white_check_mark:Succeeded
View

articles/media/bloch.svg
:white_check_mark:Succeeded
View

articles/media/bloch2.svg
:white_check_mark:Succeeded
View

articles/media/concepts_2.png
:white_check_mark:Succeeded

articles/media/concepts_3.png
:white_check_mark:Succeeded

articles/media/concepts_4.png
:white_check_mark:Succeeded

articles/media/concepts_5.png
:white_check_mark:Succeeded

articles/media/concepts_6.png
:white_check_mark:Succeeded

articles/media/concepts_7.png
:white_check_mark:Succeeded

articles/media/concepts_8.png
:white_check_mark:Succeeded

articles/media/concepts_8b.png
:white_check_mark:Succeeded

articles/media/concepts_tp.png
:white_check_mark:Succeeded

articles/media/concepts_tp2.png
:white_check_mark:Succeeded

articles/media/draper.svg
:white_check_mark:Succeeded
View

articles/media/prelude_hadamardBloch.png
:white_check_mark:Succeeded
View

articles/media/stack.svg
:white_check_mark:Succeeded
View

articles/media/teleportation.svg
:white_check_mark:Succeeded
View

articles/media/tp.svg
:white_check_mark:Succeeded
View

articles/media/tp2.svg
:white_check_mark:Succeeded
View

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

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

articles/media/README.md
77
25
2
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

articles/media/README.md
77
25
2
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Docs Build status updates of commit 9c22b65:
:warning: Validation status: warnings

File
Status
Preview URL
Details

articles/media/README.md
:warning:Warning
View
Details

articles/concepts/circuits.md
:white_check_mark:Succeeded
View

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/2.svg
:white_check_mark:Succeeded
View

articles/media/3.svg
:white_check_mark:Succeeded
View

articles/media/4.svg
:white_check_mark:Succeeded
View

articles/media/5.svg
:white_check_mark:Succeeded
View

articles/media/6.svg
:white_check_mark:Succeeded
View

articles/media/7.svg
:white_check_mark:Succeeded
View

articles/media/8.svg
:white_check_mark:Succeeded
View

articles/media/8b.svg
:white_check_mark:Succeeded
View

articles/media/Concepts1.png
:white_check_mark:Succeeded

articles/media/ItQPE.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

articles/media/beau.svg
:white_check_mark:Succeeded
View

articles/media/bloch.svg
:white_check_mark:Succeeded
View

articles/media/bloch2.svg
:white_check_mark:Succeeded
View

articles/media/concepts_2.png
:white_check_mark:Succeeded

articles/media/concepts_3.png
:white_check_mark:Succeeded

articles/media/concepts_4.png
:white_check_mark:Succeeded

articles/media/concepts_5.png
:white_check_mark:Succeeded

articles/media/concepts_6.png
:white_check_mark:Succeeded

articles/media/concepts_7.png
:white_check_mark:Succeeded

articles/media/concepts_8.png
:white_check_mark:Succeeded

articles/media/concepts_8b.png
:white_check_mark:Succeeded

articles/media/concepts_tp.png
:white_check_mark:Succeeded

articles/media/concepts_tp2.png
:white_check_mark:Succeeded

articles/media/draper.svg
:white_check_mark:Succeeded
View

articles/media/prelude_hadamardBloch.png
:white_check_mark:Succeeded
View

articles/media/stack.svg
:white_check_mark:Succeeded
View

articles/media/teleportation.svg
:white_check_mark:Succeeded
View

articles/media/tp.svg
:white_check_mark:Succeeded
View

articles/media/tp2.svg
:white_check_mark:Succeeded
View

articles/media/README.md

[Warning] Missing attribute: author. Add the current author's GitHub ID.
[Warning] Missing attribute: title. Add a title string to show in search engine results.

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

Docs Build status updates of commit 7475359:
:warning: Validation status: warnings

File
Status
Preview URL
Details

articles/media/README.md
:warning:Warning
View
Details

articles/concepts/circuits.md
:white_check_mark:Succeeded
View

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/2.svg
:white_check_mark:Succeeded
View

articles/media/3.svg
:white_check_mark:Succeeded
View

articles/media/4.svg
:white_check_mark:Succeeded
View

articles/media/5.svg
:white_check_mark:Succeeded
View

articles/media/6.svg
:white_check_mark:Succeeded
View

articles/media/7.svg
:white_check_mark:Succeeded
View

articles/media/8.svg
:white_check_mark:Succeeded
View

articles/media/8b.svg
:white_check_mark:Succeeded
View

articles/media/Concepts1.png
:white_check_mark:Succeeded

articles/media/ItQPE.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

articles/media/beau.svg
:white_check_mark:Succeeded
View

articles/media/bloch.svg
:white_check_mark:Succeeded
View

articles/media/bloch2.svg
:white_check_mark:Succeeded
View

articles/media/concepts_2.png
:white_check_mark:Succeeded

articles/media/concepts_3.png
:white_check_mark:Succeeded

articles/media/concepts_4.png
:white_check_mark:Succeeded

articles/media/concepts_5.png
:white_check_mark:Succeeded

articles/media/concepts_6.png
:white_check_mark:Succeeded

articles/media/concepts_7.png
:white_check_mark:Succeeded

articles/media/concepts_8.png
:white_check_mark:Succeeded

articles/media/concepts_8b.png
:white_check_mark:Succeeded

articles/media/concepts_tp.png
:white_check_mark:Succeeded

articles/media/concepts_tp2.png
:white_check_mark:Succeeded

articles/media/draper.svg
:white_check_mark:Succeeded
View

articles/media/prelude_hadamardBloch.png
:white_check_mark:Succeeded
View

articles/media/stack.svg
:white_check_mark:Succeeded
View

articles/media/teleportation.svg
:white_check_mark:Succeeded
View

articles/media/tp.svg
:white_check_mark:Succeeded
View

articles/media/tp2.svg
:white_check_mark:Succeeded
View

articles/media/README.md

[Warning] Missing attribute: author. Add the current author's GitHub ID.
[Warning] Missing attribute: title. Add a title string to show in search engine results.

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

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

articles/libraries/standard/algorithms.md
73
56
32
link
:white_check_mark:

articles/media/README.md
77
25
2
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

articles/libraries/standard/algorithms.md
73
56
32
link
:white_check_mark:

articles/media/README.md
78
26
2
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Docs Build status updates of commit 4252859:
:warning: Validation status: warnings

File
Status
Preview URL
Details

articles/media/README.md
:warning:Warning
View
Details

articles/concepts/circuits.md
:white_check_mark:Succeeded
View

articles/libraries/standard/algorithms.md
:white_check_mark:Succeeded
View

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/2.svg
:white_check_mark:Succeeded
View

articles/media/3.svg
:white_check_mark:Succeeded
View

articles/media/4.svg
:white_check_mark:Succeeded
View

articles/media/5.svg
:white_check_mark:Succeeded
View

articles/media/6.svg
:white_check_mark:Succeeded
View

articles/media/7.svg
:white_check_mark:Succeeded
View

articles/media/8.svg
:white_check_mark:Succeeded
View

articles/media/8b.svg
:white_check_mark:Succeeded
View

articles/media/Concepts1.png
:white_check_mark:Succeeded

articles/media/ItQPE.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

articles/media/beau.png
:white_check_mark:Succeeded

articles/media/beau.svg
:white_check_mark:Succeeded
View

articles/media/bloch.svg
:white_check_mark:Succeeded
View

articles/media/bloch2.svg
:white_check_mark:Succeeded
View

articles/media/concepts_2.png
:white_check_mark:Succeeded

articles/media/concepts_3.png
:white_check_mark:Succeeded

articles/media/concepts_4.png
:white_check_mark:Succeeded

articles/media/concepts_5.png
:white_check_mark:Succeeded

articles/media/concepts_6.png
:white_check_mark:Succeeded

articles/media/concepts_7.png
:white_check_mark:Succeeded

articles/media/concepts_8.png
:white_check_mark:Succeeded

articles/media/concepts_8b.png
:white_check_mark:Succeeded

articles/media/concepts_tp.png
:white_check_mark:Succeeded

articles/media/concepts_tp2.png
:white_check_mark:Succeeded

articles/media/draper.png
:white_check_mark:Succeeded

articles/media/draper.svg
:white_check_mark:Succeeded
View

articles/media/prelude_hadamardBloch.png
:white_check_mark:Succeeded
View

articles/media/stack.svg
:white_check_mark:Succeeded
View

articles/media/teleportation.svg
:white_check_mark:Succeeded
View

articles/media/tp.svg
:white_check_mark:Succeeded
View

articles/media/tp2.svg
:white_check_mark:Succeeded
View

articles/media/README.md

[Warning] Missing attribute: author. Add the current author's GitHub ID.
[Warning] Missing attribute: title. Add a title string to show in search engine results.

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

Acrolinx Scorecards
A minimum Acrolinx score of 80 is required.
Click the scorecard links for each article to review the Acrolinx feedback on grammar, spelling, punctuation, writing style, and terminology.

Article
Acrolinxscore
Lowestcategoryscore
Spellingissues
Scorecard
Processed

articles/concepts/circuits.md
79
57
0
link
:white_check_mark:

articles/libraries/standard/algorithms.md
73
56
32
link
:white_check_mark:

articles/media/README.md
78
26
2
link
:white_check_mark:

More information about Acrolinx

Install Acrolinx locally for VSCode
False positives or issues
Troubleshooting issues with Acrolinx

Docs Build status updates of commit 40b12f7:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

articles/concepts/circuits.md
:white_check_mark:Succeeded
View

articles/libraries/standard/algorithms.md
:white_check_mark:Succeeded
View

articles/media/1.svg
:white_check_mark:Succeeded
View

articles/media/2.svg
:white_check_mark:Succeeded
View

articles/media/3.svg
:white_check_mark:Succeeded
View

articles/media/4.svg
:white_check_mark:Succeeded
View

articles/media/5.svg
:white_check_mark:Succeeded
View

articles/media/6.svg
:white_check_mark:Succeeded
View

articles/media/7.svg
:white_check_mark:Succeeded
View

articles/media/8.svg
:white_check_mark:Succeeded
View

articles/media/8b.svg
:white_check_mark:Succeeded
View

articles/media/Concepts1.png
:white_check_mark:Succeeded

articles/media/ItQPE.svg
:white_check_mark:Succeeded
View

articles/media/QPE.svg
:white_check_mark:Succeeded
View

articles/media/README.md
:white_check_mark:Succeeded
View

articles/media/beau.png
:white_check_mark:Succeeded

articles/media/beau.svg
:white_check_mark:Succeeded
View

articles/media/bloch.svg
:white_check_mark:Succeeded
View

articles/media/bloch2.svg
:white_check_mark:Succeeded
View

articles/media/concepts_2.png
:white_check_mark:Succeeded

articles/media/concepts_3.png
:white_check_mark:Succeeded

articles/media/concepts_4.png
:white_check_mark:Succeeded

articles/media/concepts_5.png
:white_check_mark:Succeeded

articles/media/concepts_6.png
:white_check_mark:Succeeded

articles/media/concepts_7.png
:white_check_mark:Succeeded

articles/media/concepts_8.png
:white_check_mark:Succeeded

articles/media/concepts_8b.png
:white_check_mark:Succeeded

articles/media/concepts_tp.png
:white_check_mark:Succeeded

articles/media/concepts_tp2.png
:white_check_mark:Succeeded

articles/media/draper.png
:white_check_mark:Succeeded

articles/media/draper.svg
:white_check_mark:Succeeded
View

articles/media/prelude_hadamardBloch.png
:white_check_mark:Succeeded
View

articles/media/stack.svg
:white_check_mark:Succeeded
View

articles/media/teleportation.svg
:white_check_mark:Succeeded
View

articles/media/tp.svg
:white_check_mark:Succeeded
View

articles/media/tp2.svg
:white_check_mark:Succeeded
View

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

Any update here @natke ?

Yes, I do have an update - there is no solution to switch images based on the UI mode right now, so we should push these changes, and keep our eye on best practice for dark mode images.


Cannot edit files synchronized with cloud storage services

It seems like NanoLedger is unable to modify files that are synchronized with cloud storage providers (I've tried GDrive, OneDrive and others). The files can be successfully read, but not written to.
Offline availability of the files seems to have no impact.
Reproduction steps

Configure the app to use a ledger stored in a cloud service (Google Drive in my case). Transactions should be successfully loaded.
Add a new transaction, fill all the fields, and press save.
A banner saying "error while writing file" pops up (details below)

Expected behaviour
Writing to synchronized files succeeds
System information
App version: https://github.com/chvp/NanoLedger/releases/tag/v0.4.5 (and previous)
Android version: 14
Error log

java.io.FileNotFoundException: Unsupported mode: wa
 at android.database.DatabaseUtils.readExceptionWithFileNotFoundExceptionFromParcel(DatabaseUtils.java:151)
 at android.content.ContentProviderProxy.openAssetFile(ContentProviderNative.java:737)
 at android.content.ContentResolver.openAssetFileDescriptor(ContentResolver.java:1878)
 at android.content.ContentResolver.openOutputStream(ContentResolver.java:1576)
 at be.chvp.nanoledger.data.LedgerRepository.appendTo(LedgerRepository.kt:95)
 at be.chvp.nanoledger.ui.add.AddViewModel$append$1.invokeSuspend(AddViewModel.kt:142)
 at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
 at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
 at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
 at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
 at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
 at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
 at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
 at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)

I've been doing this with NextCloud and it does work. Nextcloud sometimes drops the offline availability and I have to open the app to get it to come to its senses, then switch back to NanoLedger for it to find it, but I have that trouble with the Android Nextcloud app in general. The NanoLedger side seems to work and I collect entries from a couple phones that way.

I've also been using it with nextcloud without issues. It seems like the content providers you've tried don't support appending to a file. This is annoying, but not impossible to fix. I can just do the append operation in the app and write the entire file. I assume that when syncing the entire file is pushed anyway, so this shouldn't result in any noticeable differences for the content providers that currently work.
I'll try to get to this issue sooner rather than later (this should only need a few lines of changes).

@leocencetti Any chance you'd be able to give this APK a try? You should just be able to install it above the one from f-droid.

@leocencetti Any chance you'd be able to give this APK a try? You should just be able to install it above the one from f-droid.

Hello @chvp, thanks for the quick reply! That seems to work! No error message and the file is updated online correctly.

I tested both Google Drive (the one I use primarily), OneDrive and kDrive: the first two work, the last one does not (without any error message, the transaction does not show up after saving).

I'll merge that PR and make a new release tomorrow. Not sure about kdrive. They do seem to have an open app on fdroid, so I'll try that out at some point and see if I can find something in the logs. Is kdrive a blocker for you or can I close this issue when #162 is merged?

Thanks! kDrive is a nice to have, but definitely not a blocker!


[python-package] Create Dataset from multiple data files

This PR implements creating Dataset from multiple data files.
We add a Sequence interface which requires data providers implement:

Return total number of data
Random access

so we can sample data without going through data in two round

Range access

used to read data in batch thus avoid loading all data into memory

With this interface, we can create Dataset from List[Sequence]. Also refer this to #2789 as I think this interface allows we use other types of efficient binary data format. I closed #3900 because it's an ugly hack and data loading features are much easier to implement in Python.
An example of creating Dataset from multiple HDF5 files is also given. As I mentioned in #2788, I'm more familiar with HDF5 file so it's choosen as the example.
#2031 also mentions the feature request, guess this is a common feature request.

Other potential improvements:
Sequence objects can be easily extended with support of batch reading with row numbers:
seq[[1,2,5,7,9,....]]

such, data provider could provide more efficient reading than current one-row-at-time fashion under the hood.

@shiyu1994 this PR is now ready. Are you still available for reviewing?

@cyfdecyf Thanks for your great contribution! I'm comment about my concerns. Overall the implementation is great, but there's seems to be some unnecessary changes.

@cyfdecyf Thanks for removing the debugging code. Would you like to continue to complete this PR for the validation datasets? If not, I can help to do that.

@cyfdecyf Thanks for removing the debugging code. Would you like to continue to complete this PR for the validation datasets? If not, I can help to do that.

It's on plan, will do in 10 days.

@cyfdecyf Thanks for removing the debugging code. Would you like to continue to complete this PR for the validation datasets? If not, I can help to do that.

validation dataset is completed with via https://github.com/microsoft/LightGBM/pull/4089/commits/92d0b39cbf357f482638e06e57e184a1c8f52aed
tho it seems saved binary file for validation datasets from same data are not guaranteed to stay identical.
I'm not so sure if this is an expected behavior.
Some observations:

[LightGBM] [Info] num_features_ 0
[LightGBM] [Info] num_total_features_ 10
[LightGBM] [Info] label_idx_ 0
[LightGBM] [Info] max_bin_ 0
[LightGBM] [Info] bin_construct_sample_cnt_ -1359898024
[LightGBM] [Info] min_data_in_bin_ 32723
[LightGBM] [Info] zero_as_missing_ 248
[LightGBM] [Info] has_raw_ 0

[LightGBM] [Info] num_features_ 0
[LightGBM] [Info] num_total_features_ 10
[LightGBM] [Info] label_idx_ 0
[LightGBM] [Info] max_bin_ 0
[LightGBM] [Info] bin_construct_sample_cnt_ -1359898024
[LightGBM] [Info] min_data_in_bin_ 32723
[LightGBM] [Info] zero_as_missing_ 248
[LightGBM] [Info] has_raw_ 0

Are these attributes from a validation dataset loaded from bin file?
For validation dataset, num_features_ 0 is not expected, since it should be set in Dataset::CreateValid, which is called by LGBM_DatasetCreateByReference.
https://github.com/microsoft/LightGBM/blob/f83180883a51729af9562ccac7b450c6da79a04a/src/io/dataset.cpp#L726-L728

@shiyu1994 @Willian-Zhang
#4177 fixed valid dataset with uninitialized instance variables.
num_features_ 0 seems like caused by too small sample count count. After increasing smallest sample count to 11, this problem is also solved.

@cyfdecyf Thanks a lot. We are almost ready, except for a typo issue.

@jameslamb Could you please check if your change requests have been addressed?

@jameslamb Thanks for the detailed review! I've left two review comments and will will resolve them in a few days.

@jameslamb Resolved all recent review comments and fixed CI.
Note that type hinting like following can't pass lint check.
def init_from_sample(
        self,
        sample_data: List[np.ndarray[np.float64]],
        sample_indices: List[np.ndarray[int]],

Seems like we need use the following:
import numpy.typing as npt

def init_from_sample(
        self,
        sample_data: List[npt.NDArray[np.float64]],
        sample_indices: List[np.NDArray[np.int]],

I'm not very family with Python type hinting. As numpy document says some features require Python 3.8, so I just changed type hint to List[np.ndarray].

@jameslamb Resolved all recent review comments and fixed CI.
Note that type hinting like following can't pass lint check.
def init_from_sample(
        self,
        sample_data: List[np.ndarray[np.float64]],
        sample_indices: List[np.ndarray[int]],

Seems like we need use the following:
import numpy.typing as npt

def init_from_sample(
        self,
        sample_data: List[npt.NDArray[np.float64]],
        sample_indices: List[npt.NDArray[np.int]],

I'm not very family with Python type hinting. As numpy document says some features require Python 3.8, so I just changed type hint to List[np.ndarray].

thanks very much for the investigation. I think List[np.ndarray] is ok for now.

@jameslamb Thanks for those detailed review. I've applied you suggestions and made a few small tweaks for other parts of this PR.

@StrikerRUS Thanks for the review. I've addressed your comments and pushed my changes.

@StrikerRUS Thanks for your comments.
While adding C API LGBM_SampleCount, I noticed rand.Sample may return number of values less than requested sample count.
https://github.com/microsoft/LightGBM/blob/0ca2c49424ffc93bee6b70a44c874e0db89cb490/include/LightGBM/utils/random.h#L63-L101
Guess this is why c_api.cpp has code like following
https://github.com/microsoft/LightGBM/blob/0ca2c49424ffc93bee6b70a44c874e0db89cb490/src/c_api.cpp#L1067-L1070
If my understanding to Random::Sample is correct, then I suggest update it's API doc.
After this PR is merged, the above sample indices logic can be replaced as:
auto sample_indices = CreateSampleIndices(total_nrow, config);
sample_cnt = static_cast<int>(sample_indices.size());

While adding C API LGBM_SampleCount, I noticed rand.Sample may return number of values less than requested sample count.
https://github.com/microsoft/LightGBM/pull/4089#issuecomment-865455814

@shiyu1994 Could you please take a look?

@StrikerRUS Thanks again for your review. I've pushed my changes to address all the comments.

While adding C API LGBM_SampleCount, I noticed rand.Sample may return number of values less than requested sample count.
#4089 (comment)

@shiyu1994 Could you please take a look?

I think Sample only returns K elements in expectation, but does not guarantee to return exactly K elements. We need to figure out an efficient way to Sample exactly K elements.

@shiyu1994

We need to figure out an efficient way to Sample exactly K elements.

Thanks a lot for taking a look! Do you think we need feature request for this?
Also, please update your review for this PR because there have been a lot of code changes at cpp side since your latest approval (now there are 2 new C API functions).

@StrikerRUS Thanks for the cleanup. I should have done renaming with find and replace..

@StrikerRUS Write to #2302. Sure, I'll update my review now.

@shiyu1994 Thanks for the review. Just addressed all the comments and pushed my changes.

@cyfdecyf @StrikerRUS Sorry, after a careful investigation, I found that the Sample function returns exactly K elements. I should delete the feature request. But I don't know why the sample_cnt should be recalculated as mentioned here https://github.com/microsoft/LightGBM/pull/4089#issuecomment-865455814.

Commit 8a0f07ae4b836cb1b4e0725597f387afbdac9b1d introduced sample_cnt assignement from sampled indices vector.
In line 87 in the following code, if NextShort always return a value near max_step, then we may sample less then K values.
https://github.com/microsoft/LightGBM/blob/8a0f07ae4b836cb1b4e0725597f387afbdac9b1d/include/LightGBM/utils/random.h#L82-L91

I see. But it seems that the latest version of Sample won't have such problem.
https://github.com/microsoft/LightGBM/blob/6a195a1bec42996b93eba02a511541a5615d068c/include/LightGBM/utils/random.h#L88-L95
In the loop above, each iteration a new element will be added into the set. But as #4371 pointed out, there's a bug in line 91, NextInt(0, r) should be NextInt(0, r + 1). I think we can fix this, and remove the recalculation of sample_cnt in the code in another PR.

@StrikerRUS @jameslamb Thank you both for your careful review. Can we merge this PR now?

Yeah, I believe we can continue (#4403 and https://github.com/microsoft/LightGBM/pull/4089#issuecomment-871958002) in a follow-up PRs.
@cyfdecyf Great contribution, many thanks!

Thanks for all the reviews. I'll start working on #4403 next week.

I'll start working on #4403 next week.

@cyfdecyf
Many thanks!
Also, #4450 has been merged, so please check https://github.com/microsoft/LightGBM/pull/4089#issuecomment-871958002

... and remove the recalculation of sample_cnt in the code in another PR.


Peer Feedback

[ ] In this project I create a todo list using React

Hello @JavierAybar,
Your project is complete! There is nothing else to say other than... it's time to merge it.
Congratulations! 🎉
Highlights 🚀
✅ Great historial of commits.
✅ Great work using GitFlow.
✅ No linter errors.
Cheers, and Happy coding!👏👏👏


what is .option {rvc,norvc,pic,nopic,push,pop} RISC-V options?

I have a question about a line in riscv-asm-manual, what does  .option | {rvc,norvc,pic,nopic,push,pop} | RISC-V options mean? and any documents about rvc, norvc, pic, nopic, push, pop?

There is some documentation in the GNU as manual
https://sourceware.org/binutils/docs-2.31/as/RISC_002dV_002dDirectives.html#RISC_002dV_002dDirectives
It probably should be mentioned in this manual also.

@jim-wilson thanks a lot!


Fix --port setting

Unfortunately there was an error in #13195 which set the --port
option before the settings were read. This PR fixes this by
moving applying this option to after the the settings are read
However, on looking further into this code I believe that the setPort
code was slightly odd.
Firstly, it may make sense to run the install page on a different
temporary port to the full system and this should be possible with
a --install-port option.
Secondy, if the --port option is provided we should apply it to both
otherwise there will be unusual behaviour on graceful restart
Thirdly, the documentation for --port says that the setting is
temporary - it should therefore not save its result to the configuration
(This however, does mean that authorized_keys and internal links may
not be correct. - I think we need to discuss this option further.)
Fix #13277
Signed-off-by: Andrew Thornton<EMAIL_ADDRESS>

Codecov Report

Merging #13288 into master will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #13288      +/-   ##
==========================================
- Coverage   42.16%   42.15%   -0.02%
==========================================
  Files         685      685
  Lines       75571    75575       +4
==========================================
- Hits        31867    31861       -6
- Misses      38482    38490       +8
- Partials     5222     5224       +2

Impacted Files
Coverage Δ

cmd/web.go
0.00% <0.00%> (ø)

modules/indexer/stats/db.go
52.17% <0.00%> (-8.70%)
:arrow_down:

modules/git/utils.go
73.77% <0.00%> (-3.28%)
:arrow_down:

modules/log/event.go
58.96% <0.00%> (-0.95%)
:arrow_down:

modules/git/repo.go
46.19% <0.00%> (-0.51%)
:arrow_down:

services/pull/check.go
49.63% <0.00%> (+0.72%)
: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 0800c7e...d494f2d. Read the comment docs.

🚀


WebSockets issues

Hi,
my webapp uses WebSockets by annotating the server code like this
@ServerEndpoint(value = "/websocket/myservlet")
It works perfecly when deployed on a real Tomcat server, but websockets don't work when run through the webapp-runner. Is it a known issue or am I doing something wrong?

Looks like, the issue still exists.

Easy fix for this.
Update your maven-dependency-plugin to include the tomcat-embed-websocket dependency.
<plugin> 	<groupId>org.apache.maven.plugins</groupId> 	<artifactId>maven-dependency-plugin</artifactId> 	<version>2.3</version> 	<executions> 		<execution> 			<phase>package</phase> 			<goals> 				<goal>copy</goal> 			</goals> 			<configuration> 				<artifactItems> 					<artifactItem> 						<groupId>com.github.jsimone</groupId> 						<artifactId>webapp-runner</artifactId> 						<version>${webapp-runner-version}</version> 						<destFileName>webapp-runner.jar</destFileName> 					</artifactItem> 					<artifactItem> 					    <groupId>org.apache.tomcat.embed</groupId> 					    <artifactId>tomcat-embed-websocket</artifactId> 					    <version>${tomcat.version}</version> 						<destFileName>tomcat-embed-websocket.jar</destFileName> 					</artifactItem> 				</artifactItems> 			</configuration> 		</execution> 	</executions> </plugin>
Then run webapp_runner by specifying the main class and the 2 dependencies on the classpath rather than as an executable jar.
java $JAVA_OPTS -cp tomcat-embed-websocket.jar:webapp-runner.jar webapp.runner.launch.Main --port 8080 myWarFile.war

@phowells do you think this is something webapp-runner should bundle in it's fat jar? The only reasons not to would be if the dependency is too volatile, or too uncommonly used.

@jkutner Yes, ideally webapp-runner would include the websockets jar.  That way the app would not have to worry about keeping the versions in sync.  The websockets dependency is no more volatile then the other libs in the tomcat release.  If a project has a legitimate concern or restriction on the size of the webapp-runner jar then I think it would fall to that project to create a custom build to exclude the functions they do not need.  As it is, websockets are a standard part of tomcat now so it should be included in the tomcat webapp-runner.

Fixed by fe2366190ac26bcb84638340341a937caf2fb4a1. Will be available in <IP_ADDRESS>


Failed to run build command for libc, failed to get rustc version

Checklist

[X] I've looked through the issues and pull requests for similar reports

Describe your issue
When trying to build my project at https://github.com/beeb/remote-wol-rs using cross, I am faced with the following error:
failed to run custom build command for libc v0.2.139
thread 'main' panicked at 'Failed to get rustc version', /home/valentin/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.139/build.rs:135:9
I have to use the nightly toolchain. I performed a cargo clean before building.
What target(s) are you cross-compiling for?
aarch64-unknown-linux-gnu, x86_64-pc-windows-gnu, x86_64-unknown-linux-gnu
Which operating system is the host (e.g computer cross is on) running?

[ ] macOS
[ ] Windows
[X] Linux / BSD
[ ] other OS (specify in description)

What architecture is the host?

[X] x86_64 / AMD64
[ ] arm32
[ ] arm64 (including Mac M1)

What container engine is cross using?

[X] docker
[ ] podman
[ ] other container engine (specify in description)

cross version
cross 0.2.4 (8c86a98 2023-01-06)
Example
❯ cargo build --package=remote_wol --lib --target-dir=target/front --target=wasm32-unknown-unknown --no-default-features --features=hydrate --release
// This above is required before building the main binary below, and works fine (doesn't use cross)

❯ cross build --package=remote_wol --bin=remote_wol --target-dir=target/server --no-default-features --features=ssr --release --target aarch64-unknown-linux-gnu
   Compiling libc v0.2.139
   Compiling syn v1.0.107
   Compiling futures-channel v0.3.25
   Compiling num-traits v0.2.15
   Compiling bytes v1.3.0
   Compiling futures-util v0.3.25
error: failed to run custom build command for `libc v0.2.139`

Caused by:
  process didn't exit successfully: `/target/release/build/libc-30baa802e0a1b71f/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-changed=build.rs

  --- stderr
  thread 'main' panicked at 'Failed to get rustc version', /home/valentin/.cargo/registry/src/github.com-1ecc6299db9ec823/libc-0.2.139/build.rs:135:9
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...

Additional information / notes
Another project at work (not public) cross-compiles fine on the same machine, with the same targets, even libc v0.2.139 which is the same version as above. Not sure what's different about this project.

can you run with cross build -v ..., then when you see + /path/to/docker run ... abort and copy the command line, then replace at the end -t ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main sh -c ... with -it ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main sh (the -it is important), you'll now be in a shell, from here, add to env the same thing cross does and then run
rustc --version

can you run with cross build -v ...

sure!
$ echo $PATH
/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/home/valentin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin
$ rustc --version
rustc 1.68.0-nightly (4781233a7 2023-01-16)
$

so why is it failing here
I'm not sure what's happening here, maybe something is setting RUSTC to be something else? but that shouldn't be possible imo as build scripts are contained when it comes to env vars

Not sure if that's what you meant, but that's what I get in the container:
$ echo $PATH
/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/home/valentin/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin
$ echo $RUSTC
�

So yeah something is up with the RUSTC environment variable

No, RUSTC will not be set, RUSTC is set at build time so that's not the issue, if it was it would panic on an earlier line, not 135.

Okay yep that's actually strange. It means RUSTC is correct, invoking the rustc --version command succeeds, but somehow parsing the return value fails...

can you add
[patch.crates-io.libc]
git = "https://github.com/Emilgardis/libc"
branch = "debug-output"

to your Cargo.toml, and then run with -vv, you should then see something like
Fresh bumpalo v3.12.0
       Fresh autocfg v1.1.0
       Fresh cfg-if v1.0.0
       Fresh wasm-bindgen-shared v0.2.83
       Fresh version_check v0.9.4
warning: rustc="rustc"
warning: output=Ok(Output { status: ExitStatus(unix_wait_status(0)), stdout: "rustc 1.66.1 (90743e729 2023-01-10)\n", stderr: "" })
       Fresh wasm-bindgen-backend v0.2.83
       Fresh serde_derive v1.0.152
       Fresh thiserror-impl v1.0.38
       Fresh wasm-bindgen-macro-support v0.2.83

Thanks for taking the time to solve this with me @Emilgardis here is the output with the patched libc:
The following warnings were emitted during compilation:

warning: rustc="rustc"
warning: output=Ok(Output { status: ExitStatus(unix_wait_status(32512)), stdout: "", stderr: "rustc: error while loading shared libraries: libLLVM-15-rust-1.68.0-nightly.so: cannot open shared object file: No such file or directory\n" })

error: failed to run custom build command for `libc v0.2.139 (https://github.com/Emilgardis/libc?branch=debug-output#559f49bd)`

Caused by:
  process didn't exit successfully: `/target/release/build/libc-708b5fb0a0841d73/build-script-build` (exit status: 101)
  --- stdout
  cargo:warning=rustc="rustc"
  cargo:warning=output=Ok(Output { status: ExitStatus(unix_wait_status(32512)), stdout: "", stderr: "rustc: error while loading shared libraries: libLLVM-15-rust-1.68.0-nightly.so: cannot open shared object file: No such file or directory\n" })

  --- stderr
  thread 'main' panicked at 'Failed to get rustc version', /home/valentin/.cargo/git/checkouts/libc-c7c8c88c119db40f/559f49b/build.rs:136:9
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...

What's the latest on this? I'm facing the same issue.

This might actually be a dupe of #1184? After reading https://github.com/cross-rs/cross/issues/1184#issuecomment-1375079647 I re-ordered my actions in the GitHub workflow (build ARMv7 after ARMv8), and now the build succeeds.

Yes! The error you're reporting is not this issue, it's #724 which has another cause; build scripts being ran on the host system which has a higher glibc