Fix RDS database name in graphite.

Instead of dbinstance:rds-instance-name now we can have only rds-instance-name.

Sounds reasonable to me. Squash merging to get cleaner commits.


Provide executable file for mac , windows and unix-like

i think it should provide executable file for some mainstream platform, so user can run and use this app  quickly if they are not go language expert

No, if you want, please package it into a docker image, not in the source code, the project is for source code not binary. closing this, see #52


allow access to azure services

Does "allow access to azure services" mean access to my db from any service/vm created by any azure user in any subscription in any Azure AD? If so, why would azure do that? Shouldn't this option be restricted only to the azure services that enable the features of azure db (like for example, import/export)?

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

ID: 2eb5097c-fc6e-f0a5-07a6-a84e05aa0bb6
Version Independent ID: 1004e6b8-344f-6fdc-391a-35c540f4bf9d
Content: Virtual Network service endpoints and rules for Azure SQL Database and SQL Data Warehouse
Content Source: articles/sql-database/sql-database-vnet-service-endpoint-rule-overview.md
Service: sql-database
GitHub Login: @oslake
Microsoft Alias: moslake

@jgallucci32 Thanks for your feedback! We will investigate and update as appropriate.

@ravikiran-srini “Allow access to Azure services” Yes/No button on the portal (Firewall settings page). Please note, enabling this feature would allow any traffic from resources/services hosted in Azure (not just your Azure subscription) to access the database.
SQL databases are protected by a firewall in Azure. By default, all connections to the server and the databases inside the server are rejected except for connections from other Azure services.
The most secure configuration is to set 'Allow access to Azure services' to OFF. If you need to connect to the database from an Azure VM or cloud service, you should create a Reserved IP (classic deployment) and allow only the reserved IP address access through the firewall. If you are using the Resource Manager deployment model, a dedicated Public IP address is assigned to the resource, and you should allow this IP address through the firewall.
For more details, refer “Azure SQL Database and SQL Data Warehouse firewall rules”.

@ravikiran-srini
We will now proceed to close this thread. If there are further questions regarding this matter, please comment and we will gladly continue the discussion.

Setting Allow access to Azure services button on the Portal (Firewall settings page) to ON shall allow traffic to/from  resources ( e.g. Azure VM) or PaaS services( e.g. Web App) hosted inside Azure - within same subscription or from another subscription as well.


修coverage的路徑

Coverage remained the same at 83.582% when pulling 75fbce4c61720e52d9dd2f368120f01db33abaa7 on 修coverage的路徑 into 34176aea483f79c1475d9fa591d79de32c959a9d on master.

Coverage remained the same at 83.582% when pulling 1272d50e02fa43e41688f37da2cf6272e36f0e4c on 修coverage的路徑 into 34176aea483f79c1475d9fa591d79de32c959a9d on master.


Rename actions to be a bit more accurate

Renames the stages in the github actions to be a bit more accurate on what is being ran

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.


LHN - #announce room and that workspace policy chat didn't get archived after Delete the Workspace

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

Action Performed:
Precondition:

2 accounts are needed for the following tests, 1 Admin and 1 user
With one of the accounts, log in, create a workspace and invite the other account to the workspace. We will refer to the invitee as the employee.
Send a message in the all the workspace rooms (#announce and #admins) and in each workspace's chats.

Log in with the same workspace admin account on the main device and the employee in the secondary device
Navigate to a Workspace settings
Disable the internet connection in the device
Delete the workspace by click/tap on the 3 dots > Delete workspace

Expected Result:
The employee lost access to the #announce room and that their workspace policy chat got archived.
Actual Result:
#announce room and that workspace policy chat didn't get archived after delete the Workspace
Workaround:
Unknown
Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: <IP_ADDRESS>
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers<EMAIL_ADDRESS><EMAIL_ADDRESS>Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/93399543/196837612-a466489f-f8bc-43b5-93be-435b64b57084.mp4
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
View all open jobs on GitHub

@kbecciv quick clarification. For steps 2-4 are taken from the admin account, correct?
And for the expected result

The employee lost access to the #announce room and that their workspace policy chat got archived.

@NikkiWines Yes, correct steps 2-4 are taken from admin account.
Expected results in Secondary device (Employee):
Verify that the employee lost access to the #announce room and that their workspace policy chat got archived.
Sending you TR link if its easy to follow the steps https://expensify.testrail.io/index.php?/tests/view/2894431.

@kbecciv I think the Secondary device - Employee steps are in TR a bit unclear.
We cannot expect the employee to lose access to the #announce room while the admin is offline because the admin's actions won't be recorded on the server until they come back online. The tests for Secondary device - Employee should be performed once the admin is online.

Issue not reproducible during KI retests. (First week)

@kbecciv I don't think this is actually a bug. Closing.


Use of NSUserDefaults in testing

Ash had an example with a stubbedUserDefaults in his Emerging Best Practices talk so I was curious if it was used in eidolon.
From looking at the code the approach is to not have a StubUserDefaults class. Instead I see NSUserDefaults(), rather than NSUserDefaults().sharedInstance(), used in the tests.
For example in ArtsyProviderTests:
// We fake our defaults to avoid actually hitting the network
defaults = NSUserDefaults()
defaults.setObject(NSDate.distantFuture(), forKey: "TokenExpiry")
defaults.setObject("Some key", forKey: "TokenKey")

This approach is very intriguing because it bypasses the work of writing and using a StubUserDefaults class. Why did you go with this approach? Do you have any examples of doing dependency injection with the NSUserDefaults() as the argument?
One downside I see is that you have to set up your faked defaults for every test, but this is not something I'm doing with my StubbedUserDefaults anyway.

The idea of stubbed user defaults might not be used in Eidolon extensively, but it's used in our other apps. Energy uses a global NSUserDefaults replacement provided by our testing components library Forgeries. This means no tests can ever access the dev's nsuserdefaults. However even with that, we still DI in our defaults whenever we can.
DI-ing in the defaults like we do in Eidolon is generally what we do in all apps as a best practice to make it easy to test, especially anything that is Apple's domain and it's normal to access as a singleton. We have a problem with this on Eigen where-in running the tests will log out the developer because it messes around with the real NSUserDefaults and NSFileManager. It's harder to make these changes once you've got hundreds of LOC though.

Yeah, I hope I don't give you the impression that we have this all figured out 😅 We're learning as we go, and in our four iOS apps, you'll find four different approaches to almost everything. The reality is, sometimes we find a good practice in a new app but don't have time to go back and rewrite old apps to use it. Also, the stubbed user defaults was a handy example I used in that talk because most devs are familiar with it.
I hope that helps, please let us know if we can clarify anything!


upgrade to 0.30.0 k8s libs

we are ready to upgrade to 0.30.0 k8s libs, but will need open-policy-agent/gatekeeper to upgrade, which depends on the OPA frameworks package.
the current PR for OPA Frameworks upgrade to 0.30.0 is https://github.com/open-policy-agent/frameworks/pull/435

@tbarnes94


Remove class collaboration diagrams from user-facing docs

These are no useful to user, and their usefulness to developers is even doubtful.  For an example:
https://coryan.github.io/google-cloud-cpp/classRowReaderTest.html

set(DOXYGEN_COLLABORATION_GRAPH NO)
set(DOXYGEN_INCLUDE_GRAPH NO)

i am checking if above two variables can remove collaboration diagram and include graph in html. I tried this with docker image of fedora with GENERATE_DOCS=yes.
But classRowReaderTest.html  is not generated at all.
or is it due to the fact that testing is in exclude pattern,
set(DOXYGEN_EXCLUDE_PATTERNS "*/client/internal/*;*/client/testing/*;*/*_test.cc")

Need to set following in DocxygenCommon.cmake
set(DOXYGEN_CLASS_GRAPH NO)
set(DOXYGEN_COLLABORATION_GRAPH NO)
set(DOXYGEN_INCLUDE_GRAPH NO)
set(DOXYGEN_INCLUDED_BY_GRAPH NO)

Please see for further reference:-
https://www.stack.nl/~dimitri/doxygen/manual/config.html#cfg_class_graph
I will raise a PR with the fix.

#397 also fixes this issue, is there anything else to be done here?

Indeed, #397 also fixed this, I just did not use the magic words in the commit message.


🔨 Update code formatting and linting configurations

Pydantic uses single quotes.

Pydantic uses single quotes.

ah just fix it https://github.com/pydantic/pydantic-extra-types/pull/148


RNDeviceInfo is null

Bug
Trying to run a clean project on Android causes the red error screen with the message:
Error: @react-native-community/react-native-device-info: NativeModule.RNDeviceInfo is null.

Environment info
React native info output:
System:
    OS: macOS Mojave 10.14.6
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
    Memory: 154.95 MB / 16.00 GB
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 10.14.1 - ~/.nvm/versions/node/v10.14.1/bin/node
    Yarn: 1.17.3 - ~/.nvm/versions/node/v10.14.1/bin/yarn
    npm: 6.11.3 - ~/.nvm/versions/node/v10.14.1/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: iOS 13.0, DriverKit 19.0, macOS 10.15, tvOS 13.0, watchOS 6.0
    Android SDK:
      API Levels: 26, 28, 29
      Build Tools: 28.0.3, 29.0.2
      System Images: android-29 | Intel x86 Atom, android-29 | Intel x86 Atom_64, android-29 | Google APIs Intel x86 Atom, android-29 | Google APIs Intel x86 Atom_64, android-29 | Google Play Intel x86 Atom, android-29 | Google Play Intel x86 Atom_64
  IDEs:
    Android Studio: 3.5 AI-191.8<IP_ADDRESS>91312
    Xcode: 11.0/11A420a - /usr/bin/xcodebuild
  npmPackages:
    react: 16.9.0 => 16.9.0
    react-native: 0.61.1 => 0.61.1

Library version: 3.1.4
Steps To Reproduce

create clean project with (RN 0.61.1):

react-native init test

yarn add react-native-device-info
cd ios && pod install && cd .. (with success message)
add code:

import DeviceInfo from 'react-native-device-info';
...
DeviceInfo.isTablet().then(isTablet => {
    console.log(isTablet);
});

Project presents red screen on Android.
Project runs just fine on iOS.

(Oddly enough, if I perform the manual linking steps [despite running RN0.61.1] the Android project runs fine then)
Describe what you expected to happen:

Both iOS and Android runs should fine and log wether if the device is a tablet or not.

Can you reproduce it by cloning this repo and running the example? Because I usually nuke + refresh the example programmatically each time and it runs fine...
https://github.com/react-native-community/react-native-device-info/blob/master/refresh-example.sh

Your example seems to run without problems.
I'll continue to investigate further.
Thanks for your time.

I'm still getting the issue and tried with refresh install, any help is appreciated.

@sagargulati you are getting the same issue, while attempting to reproduce with the example? Because so far the example seems to work, indicating the module works fine, so these are project integration issues (which are difficult to troubleshoot because each project is different...)

I'm getting the same issue, but running from xcode worked, when running from react-native run-ios it doesn't instance the object properly.

@mikehardy : Yes, similar, it shows me the same error in Android, haven't tested with iOS

i'm also running into same bug iOS

posting me-toos does not work. I need someone to clone this repo, run the example, and tell me it does not work.
Here's what I did
git clone git@github.com:react-native-community/react-native-device-info.git
cd react-native-device-info
./refresh-example.sh
cd example
yarn ios

It works. Any evidence to the contrary I/we all over it. In the meantime, something is wrong with project integration at the project-specific level and that's not something we can control or even really troubleshoot effectively, which is why we provide not only an example but an example built by a script you may inspect to make sure you can reproduce actual example construction
I'm not sure what more I/we can do?

Im having an issue trying to use a private repo with the new testing-library for react-native (.59)
My hunch is that if we can upgrade to .61 the issue will go away, ive tried performing manual link, xcode build, pod install etc

@mikehardy I was having issues getting this package to work in Android with RN 0.61.0.  Which lead me to this issue page.
I tested the example project in both ios and android:
git clone git@github.com:react-native-community/react-native-device-info.git
cd react-native-device-info
./refresh-example.sh
cd example
yarn ios (yarn android)

Both are platforms are showing the following error:
TypeError: null is not an object (evaluating '_RNGestureHandlerModule.default.Direction')

<global>
    Directions.js:3:38
loadModuleImplementation
    require.js:322:6
<global>
    GestureHandler.js:2
loadModuleImplementation
    require.js:322:6
<global>
    Swipeable.js:10
loadModuleImplementation
    require.js:322:6
<global>
    index.js:1
loadModuleImplementation
    require.js:322:6
<global>
    index.js:1
loadModuleImplementation
    require.js:322:6
guardedLoadModule
    require.js:201:45
global code
    index.bundle?platform=ios&dev=true&minify=false:232245:4

Seems another major bug with AndroidX and GCM for device-info

@sagargulati lacks any context details required to troubleshoot
I think what you are referring to is #802 fixed by #804
released just now as @next on NPM or 4.0.1-beta.2
Use the new iid-specific version property in your android/build.gradle::ext{} block and you can specify IID and Play Services versions independently, as they are versioned separately in the AndroidX world (IID is at 17.0.0 while Play Services moved to 17.1.0), leading to this error

Same trouble! Does anyone know how to solve it?

Error: @react-native-community/react-native-device-info: NativeModule.RNDeviceInfo is null. To fix this issue try these steps:
• For react-native <= 0.59: Run react-native link react-native-device-info in the project root.
• Rebuild and re-run the app.
• If you are using CocoaPods on iOS, run pod install in the ios directory and then rebuild and re-run the app. You may also need to re-open Xcode to get the new pods.
If none of these fix the issue, please open an issue on the Github repository: https://github.com/react-native-community/react-native-device-info
nativeInterface.ts:20:10 loadModuleImplementation require.js:322:6 index.ts:5 loadModuleImplementation require.js:322:6 Login.js:12 loadModuleImplementation require.js:322:6 route.js:5 loadModuleImplementation require.js:322:6 App.js:3 loadModuleImplementation require.js:322:6 index.js:6 loadModuleImplementation require.js:322:6 guardedLoadModule require.js:201:45 global code index.bundle?platform=ios&dev=true&minify=false:166842:4

same issue here

Same issue

Any time you use a native module in react-native and you integrate it improperly, this will happen.
Always look for the example project in any native react-native module you use, and run it, to prove it works. Once you see it works, then if the module is null in your project you have a project-specific issue and you need to integrate it correctly.
My previous comment includes a script that shows how to do so


'App\LocalizationSetting' not found

installed in a windows env

Having the same issue running on XAMPP: Trying to get property 'language' of non-object
App::setLocale($localization_setting->language);


Changing the title of the page

Saw the website and it has React App as title. I can change the title.

Okay @tokyo3001


The dart analyze CLI supports only one directory while the older dartanalyzer supported multiple ones

Dart SDK Version (dart --version): 2.12.1
Whether you are using Windows, MacOSX, or Linux (if applicable): Linux (but probably applies to others)

The old dartanalyzer CLI supports passing multiple directories to analyze. dart analyze does not.
Is this an intentional change or something that was missed in the new CLI ?
I found this out when working on the dart-sass package. The CI setup still uses the dartanalyzer CLI due to that. Locally, I tried using the new one as dartanalyzer is not in the PATH by default when installing the ubuntu package.

@devoncarew


Handle captured identifiers in format strings

Since 1.58 Rust allows println!("Hello, {person}!"); style strings (documentations). I would like to propose cargo fmt to automatically convert println!("Hello, {}!", person); and other format!-like calls to the inlined versions, either by default, or, at least initially, as an opt-in.

Original code
After cargo fmt
Notes

println!("{}", a)
println!("{a}")
positional in-lining

println!("{a}", a=b)
println!("{b}")
named param

println!("{a:0$}", width)
println!("{a:width$}")
width param

println!("{a:.0$}", prec)
println!("{a:.prec$}")
precision param

I wonder if rustfix might be a better venue for such transformations

I wonder if rustfix might be a better venue for such transformations

I just looked at rustfix readme:   The magic of rustfix is entirely dependent on the diagnostics implemented in the Rust compiler (and external lints, like clippy).
My understanding is that rustfix won't actually decide anything, but rather use whatever suggestion is made by rustc or clippy. I am not sure this type of fix would belong in the rustc itself. Do you think clippy would be a better target?

I think clippy would probably be the best place to start, just to see if they happen to already have a lint or if they have one planned. If they do, then that's something rustfix could pick up.

Thanks @calebcartwright , turns out clippy already has a similar (smaller-scoped) request, so closing in favor of https://github.com/rust-lang/rust-clippy/issues/8368

Perfect, thanks for sharing!


Improving the catalyst installation guide: prerequsite order and OS specific subsections

Context: Improvements to the installation documentation
Description of the Change:

It is a bit confusing for the
pip install -r requirements.txt
to come before the
git clone --recurse-submodules --shallow-submodules https://github.com/PennyLaneAI/catalyst.git
in the installation guide, since a lot of people don’t carefully read anything before the “git clone”.

Added a summary section at the top of the 'compile from source' section for each OS, allowing users to easily see together all the commands to run.

Benefits: See above
Possible Drawbacks:
Related GitHub Issues:
[sc-54448]

Thanks @paul0403! This is a nice improvement.
I have one question, however -- does the 'quick' install from source guide make any parts of the old, more detailed guide redundant? I just had a read through, and it looks like roughly the same information in most places (cc @dime10)

Yes I think it does make some parts redundant (that is it's purpose after all?). The question is whether we are okay with most people skipping the existing (that is very likely to happen if an easier option is presented to them), and whether that will lead to more issues 🤔

Thanks @paul0403! This is a nice improvement.
I have one question, however -- does the 'quick' install from source guide make any parts of the old, more detailed guide redundant? I just had a read through, and it looks like roughly the same information in most places (cc @dime10)

Yes I think it does make some parts redundant (that is it's purpose after all?). The question is whether we are okay with most people skipping the existing guide (that is very likely to happen if an easier option is presented to them), and whether that will lead to more issues 🤔

I think there is no way avoiding the users skipping the detailed guide if they really want to, and it doesn't matter how we design the summary section. As long as it's there, no matter the form, some users will tend to skip the detailed sections. The most we can do is in the summary section say something like "we strongly recommend reading the detailed guide".

The new "Minimal" section separates the commands for building under Linux and MacOS, but they only differ in the way the dependencies are installed. After that, cloning and building are the same commands. As a user I would prefer to spot immediately what are the pieces that remain unchanged through different OSs. Repeating those commands seems to me redundant.
The comments should be taken outside the code boxes and shown as normal step text. That way the copied code will not contain text that some shells might interpret or not as a comment.
I would prefer having something like:

General installation steps
      Installation under Linux
            Minimal guide
            Extended guide
      Installation under MacOS
            Minimal guide
            Extended guide

That way the user does not lose context by scrolling further down if he wants to go beyond the minimal guide.
What do you think?

I agree with Paul's point on the structure.


Update index.html

Related Issue

Issue No. #

Changes you made :

Describe the changes :

Check list :

[x] I have followed code style of this project.
[x] I have read the CONTRIBUTING document.
[x] All the aspects mentioned in the issue are covered.
[x] The website is properly working after my changes.
[x] 🌟 ed the repo

Type of change :

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)

Output Screenshots :

changes in footer section

@tannu610 Thanks for this PR
But you need to create an issue first in order to work on a project.
Also there have been several improvements on the footer.
If you want to add anything new plz mention it in your issue.
Thank you 👍


Update the readme

Add all the new options that are in the action.yml file

Better yet, find or make a simple way to auto-update them given they have descriptions.

Mustache can't do this as it can't get the keys out, Handlebars can, but we'd have to use yq or something to convert the YAML to JSON first:
https://handlebarsjs.com/playground.html#format=1&currentExample={"template"%3A"%23 {{name}}\n{{description}}\n{{%23each inputs}}\n\n%23%23%23 Parameter%3A {{%40key}}\n{{this.description}}\n{{%2Feach}}"%2C"partials"%3A[]%2C"input"%3A"{\n  name%3A 'Codespell with annotations'%2C\n  description%3A 'Codespell with annotations for Pull Request'%2C\n  inputs%3A {\n    'check_filenames'%3A {\n    \tdescription%3A \"foo\"\n    }%2C\n    'check_bar'%3A {\n    \tdescription%3A \"bar\"\n    }%2C\n  }%2C\n}\n"%2C"output"%3A"%23 Codespell with annotations\nCodespell with annotations for Pull Request\n\n%23%23%23 Parameter%3A check_filenames\nfoo\n\n%23%23%23 Parameter%3A check_bar\nbar\n"%2C"preparationScript"%3A"%2F%2F Handlebars.registerHelper('loud'%2C function(string) {\n%2F%2F    return string.toUpperCase()\n%2F%2F })%3B%5Cn%22%2C%22handlebarsVersion%22%3A%224.7.7%22%7D


Tweak building documentation

This should fix #1802.
Tested on my fork: https://johntitor.github.io/libc/

@JohnTitor: no appropriate reviewer found, use r? to override


feat(#126): Remove RuleSuppressed from RuleAllTestsHaveProductionClass

Remove RuleSuppressed from RuleAllTestsHaveProductionClass.
Closes: #126

PR-Codex overview
This PR focuses on adding the @ToString annotation to the ComplaintLinked class and refactoring the inspection method in the Cop class.
Detailed summary

Added @ToString annotation to ComplaintLinked class.
Refactored inspection method in Cop class to use method references and stream operations.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@rultor merge

@rultor merge

@volodya-lombrozo OK, I'll try to merge now. You can check the progress of the merge here

@rultor merge

@volodya-lombrozo Done! FYI, the full log is here (took me 3min)


updated umap to include epoch count

Codecov Report

Merging #189 into master will decrease coverage by 0.08%.
The diff coverage is 28.57%.

@@            Coverage Diff            @@
##           master    #189      +/-   ##
=========================================
- Coverage   79.98%   79.9%   -0.09%
=========================================
  Files          86      86
  Lines        3473    3479       +6
  Branches      746     751       +5
=========================================
+ Hits         2778    2780       +2
- Misses        693     697       +4
  Partials        2       2

Impacted Files
Coverage Δ

src/singlepage/UMAPVisualization.tsx
0% <0%> (ø)
:arrow_up:

src/component/widget/ComponentDock.tsx
95% <66.66%> (-5%)
:arrow_down:

Continue to review full report at Codecov.

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


RCP API Request: sui_getPackagesPublishedByAddress

There is currently no way to get package details other by providing the package ID to sui_getObject but the owner address is Immutable which can not be provided through any JSON RPC API
However; like sui_getObjectsOwnedByAddress it would be convenient for SDK and other uses to get the collection of packages by address in one call.

In the meantime, I think https://docs.sui.io/sui-jsonrpc#sui_getEventsBySender, then filtering the results by PublishEvent is a workaround for this.

Yes, I looked at that but it would mean running constantly else potential for missing the event right? I don't see any option to back process missed events unless I'm missing something?

@FrankC01 docs.sui.io/sui-jsonrpc#sui_getEventsBySender is a historical event query API(as opposed to event subscription API), and does not require you to run constantly.

Ahh, that works and thanks. @sblackshear apologies as I associated your response with event processing.


Maintain original key in allocate()

@Freeaqingme Can you add a test for this?

@frederikbosch Sure thing. Please see the updated change set.


TCVP-2834 add configuration item to enable quorum queues

Description
This PR includes the following proposed change(s):

adds a UseQuorumQueues configuration property that defaults to false.
if UseQuorumQueues is true, MassTransit is configured to set the queues to be quorum queues required for clustered configuration.
Queues must be declared as quorum queues when created

Type of change

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Refactoring / Documentation
[ ] Version change

if your change is a breaking change, please add breaking change label to this PR
How Has This Been Tested?
I cleared my rabbitmq storage in docker, restarted the workflow service and rabbitmq and observed the queues being created as quorum queues.
Does the change impact or break the Docker build?

[ ] Yes
[x] No

If Yes: Has Docker been updated accordingly?

[ ] Yes
[ ] No

Checklist:

[x] My code follows the style guidelines of this project
[x] I have performed a self-review of my own code
[ ] I have made corresponding changes to the documentation
[ ] New and existing unit tests pass locally with my changes

Not sure if it is line endings causing these files to look like the entire file changed. I updated the gitattributes file to maybe fix?


Create CONTRIBUTING.md

Feature Request
Name: CONTRIBUTING.md
About: Create a guide for contributors to get started.
Make sure to include a link to the discord.

Create or grab an issue
Fork and create your branch
Setup pre-commit to ensure your PR passes tests
Submit your PR

Style guide to keep code consistent between all developers.

Closed in #44


Enable master branch protection

      This repository does not seem to have master branch
      protection enabled, at least in the way I'm expecting.
      I was hoping for:

      - master branch protection
      - requiring at least one code reviewer
      - requiring at least two status checks
      - enforcing rules for admins

      Please turn it on!

@dazuma , friendly ping to take a look and fix it.

Ping!
@viacheslav-rostovtsev , should this be on us (actools@) to enable this protection since we manage this repo? Please advice. We have to enable this otherwise it would gate Justin Beckwith's proposal to open up the googleapis/ for googlers. Thanks!

Daniel, is there a reason to not enable branch protection today?

It looks like someone already enabled it for this repo.


warning: manually calling a React.PropTypes validation function

Looks like update is needed to start using prop-types instead of React.PropTypes. I receive around 40 warnings or so like the one below.
React.PropTypes validation function for the distanceToMouseprop onGoogleMapMarkers. This is deprecated and will not work in production with the next major version. You may be seeing this warning due to a third-party PropTypes library. See https://fb.me/react-warning-dont-call-proptypes for details.

Seems like you need to update this library, what version are u using now?

because its already fixed https://github.com/istarkov/google-map-react/blob/master/src/google_map_markers.js#L4

And what about Flow?

No flow there ;-)

I mean ability to remove 'prop-types' and use Flow. For now I can't remove 'prop-types' from my package.json and it's very sad.

I use flow for other projects, but not sure I want to somehow use it here with current API. I have an idea to rewrite it, but have no time

Ok, I understood you. Now I removed 'prop-types' from my package.json and I have error:
Module not found: Can't resolve 'prop-types' in '/home/binjo/WebstormProjects/gitlab/react.ui/node_modules/google-map-react/lib'
 @ multi ./~/react-styleguidist/lib/index ./~/react-dev-utils/webpackHotDevClient.js
Error in ./~/google-map-react/lib/google_map_markers.js
Module not found: Can't resolve 'prop-types' in '/home/binjo/WebstormProjects/gitlab/react.ui/node_modules/google-map-react/lib'
 @ multi ./~/react-styleguidist/lib/index ./~/react-dev-utils/webpackHotDevClient.js

Any ideas?

@binjospookie Had same problem and that broke our build pipelines. Seems that prop-types is only presented as devDependencies of google-map-react and what i have understood npm doesn't install those automatically. Now google_maps_markers.js tries to use them in with distribution dependencies only and this causes error.
I'm not 100% sure but moving prop-types from devDependencies to dependencies should do the trick. @istarkov What do you think?

I think we should look how its done by other libraries like react-virtualized, react-router, react-redux and just do the same ;-) On mobile now, so have no chance to look fast how its done there

@istarkov sorry for the slow response. I switched to using static maps because of project demands, so forgot about this. I was using 0.24.0.

I'm having this same issue on version 0.24.0, My Current fix is to add the prop-types package to my app the keep dev going.

@zacck @Matzu89 @Snaddu @binjospookie This is fixed guys:
https://github.com/istarkov/google-map-react/blob/master/src/google_map_markers.js#L32
Closing the issue, please let me know if you need more help or create a new issue, thanks!


How do I animate between root screens.

I have an app coordinator that has several child coordinators. I show each child by replacing the root of the app coordinator‘s routes property. Right now it immediately switch between them. Is it possible to dissolve between them?

I figured it out. I wrapped the assignment of routes with withAnimation.
       withAnimation {
            routes = [.root(.login(loginCoordinator))]
        }

Thanks, glad you figured it out @kgrigsby-codesmith. Incidentally, I'd like to be able to offer some utility methods in this library to swap the top screen for a new one with a custom transition, but in my experimentation, SwiftUI's .transition(...) modifier only seems to work for the root screen.


Update layout for tablets

Tablet in landscape mode should display normal desktop version, just with tighter FAQ column: https://zpl.io/a74G9qp
Tablet in portrait will be similar to mobile in landscape - intro & login screens centered  https://zpl.io/begmngq but images and texts like "We're here for you. First, let's narrow down your request." will be shown

Hmm...there is some mistake. On iPad Pro it shows me odd version of Smart Faq.

I tried it on Safari, Chrome and Opera mini last versions. Can it be because I tested this on iPad Pro, or? @kantord @jalbertsr

Breakpoint are not working properly, see strange error on issues https://github.com/kiwicom/smart-faq/issues/538, https://github.com/kiwicom/smart-faq/issues/512

👍


Adding TopoJSON map generator link in README

TopoJSON-map-generator is a bash script that generates maps (with lakes, provinces, embeded projection, with/without antarctica) using one command line.

Probably better to link from the wiki than this project: https://github.com/topojson/topojson/wiki


How to split a FieldGroup into multiple rows when using display-flex?

@aitboudad
I am using ngx-formly/core and /material version 5.5.7 with Angular 7.2.15
Appreciate all the hard work that is going into ngx-formly.
I am looking for some help here.
I would like to split a FieldGroup into multiple rows. I have tried the below example, but it results in the following image.

I would like to see the second repeat section as a new row as compared to where it is today.
Also for some reason when using Flex-8 (custom CSS - see below) the field stretches to the entire row when it is the only field. Work fine when there are 2 or more fields.
`.flex-8 {
// flex-flow: flex-direction flex-wrap;
// flex-direction: row ;
// flex-wrap: wrap;
.mat-form-field-infix {
width: 200px;
// min-width: 250px;
// max-width: 400px;
}
}
`
Looking forward to your guidance.
Thanks
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
`  deviceAlertFields: FormlyFieldConfig[] = [
{
fieldGroupClassName: 'display-flex',
key: 'devicesensors',
type: 'repeat',
wrappers: ['panel'],
templateOptions: { addText: 'Add Sensor',  },
fieldArray: {
fieldGroup: [
{
fieldGroupClassName: 'display-flex',
fieldGroup: [
{ type: 'input', key: 'sensorid', className: 'flex-8', templateOptions: { label: 'Sensor', placeholder: 'Sensor ID ', type: 'string', appearance: 'outline', required: true },
.........  code deleted .......... },
{ type: 'input', key: 'unit', className: 'flex-8', templateOptions: { label: 'Unit', placeholder: 'unit of measurement ', type: 'string', appearance: 'outline', required: true } },
{ type: 'input', key: 'min', className: 'flex-8', templateOptions: { label: 'Min Value', placeholder: 'Min Value', type: 'string', appearance: 'outline', } },
{ type: 'input', key: 'max', className: 'flex-8', templateOptions: { label: 'Max Value', placeholder: 'Max Value', type: 'string', appearance: 'outline',  } },
{
fieldGroupClassName: 'display-flex,',
key: 'alerts',
type: 'repeat',
          wrappers: ['panel'],
          templateOptions: { addText: 'Add Alert' , },
          fieldArray: {
            fieldGroup: [
              { type: 'select', key: 'alertlevel', className: 'flex-8', templateOptions: { label: 'Alert Level', placeholder: '', //multiple: true, enableSelectAll: true,
                options: [
                  { label: 'Major', value: 'Major' },
                  { label: 'Minor', value: 'Minor' },
                  { label: 'Critical', value: 'Critical' },
                ],
                }
              },
              { type: 'input', key: 'high', className: 'flex-8', templateOptions: { label: 'High Value', placeholder: 'Max Value', type: 'string', appearance: 'outline',  } },
              { type: 'input', key: 'low', className: 'flex-8', templateOptions: { label: 'Low Value', placeholder: 'Max Value', type: 'string', appearance: 'outline',  } },
            ]
          },
        },
        ],
      },
    ],
  },
},`

Could you set it up in a stackblitz?
On Thu, Jan 9, 2020, 12:14 PM argupta23<EMAIL_ADDRESS>wrote:

@aitboudad https://github.com/aitboudad
Appreciate all the hard work that is going into ngx-formly.
I am looking for some help here.
I would like to split a FieldGroup into multiple rows. I have tried the
below example, but it results in the following image.
[image: image]
https://user-images.githubusercontent.com/39933836/72092585-3f077700-32d8-11ea-8d8a-d2c214b37cfb.png
I would like to see the second repeat section as a new row as compared to
where it is today.
Also for some reason when using Flex-8 (custom CSS - see below) the field
stretches to the entire row when it is the only field. Work fine when there
are 2 or more fields.
`.flex-8 {
// flex-flow: flex-direction flex-wrap;
// flex-direction: row ;
// flex-wrap: wrap;
.mat-form-field-infix {
width: 200px;
// min-width: 250px;
// max-width: 400px;
}
}
`
Looking forward to your guidance.
Thanks
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
` deviceAlertFields: FormlyFieldConfig[] = [
{
fieldGroupClassName: 'display-flex',
key: 'devicesensors',
type: 'repeat',
wrappers: ['panel'],
templateOptions: { addText: 'Add Sensor', },
fieldArray: {
fieldGroup: [
{
fieldGroupClassName: 'display-flex',
fieldGroup: [
{ type: 'input', key: 'sensorid', className: 'flex-8', templateOptions: {
label: 'Sensor', placeholder: 'Sensor ID ', type: 'string', appearance:
'outline', required: true },
......... code deleted .......... },
{ type: 'input', key: 'unit', className: 'flex-8', templateOptions: {
label: 'Unit', placeholder: 'unit of measurement ', type: 'string',
appearance: 'outline', required: true } },
{ type: 'input', key: 'min', className: 'flex-8', templateOptions: {
label: 'Min Value', placeholder: 'Min Value', type: 'string', appearance:
'outline', } },
{ type: 'input', key: 'max', className: 'flex-8', templateOptions: {
label: 'Max Value', placeholder: 'Max Value', type: 'string', appearance:
'outline', } },
{
fieldGroupClassName: 'display-flex,',
key: 'alerts',
type: 'repeat',
      wrappers: ['panel'],
      templateOptions: { addText: 'Add Alert' , },
      fieldArray: {
        fieldGroup: [
          { type: 'select', key: 'alertlevel', className: 'flex-8', templateOptions: { label: 'Alert Level', placeholder: '', //multiple: true, enableSelectAll: true,
            options: [
              { label: 'Major', value: 'Major' },
              { label: 'Minor', value: 'Minor' },
              { label: 'Critical', value: 'Critical' },
            ],
            }
          },
          { type: 'input', key: 'high', className: 'flex-8', templateOptions: { label: 'High Value', placeholder: 'Max Value', type: 'string', appearance: 'outline',  } },
          { type: 'input', key: 'low', className: 'flex-8', templateOptions: { label: 'Low Value', placeholder: 'Max Value', type: 'string', appearance: 'outline',  } },
        ]
      },
    },
    ],
  },
],

},
},`
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/ngx-formly/ngx-formly/issues/2019?email_source=notifications&email_token=ADB4XNOJAMILRFYSTRMCY53Q45SRNA5CNFSM4KE4NZD2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IFEKFKA,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ADB4XNPF5NWDF2UOZJACHYTQ45SRNANCNFSM4KE4NZDQ
.

@kenisteward, @aitboudad
Here is a stackblitz I created.
https://angular-ipvwaf.stackblitz.io
Please Click on Add-Sensor and you will see Add-Alert appear to the right which I would like to see as a new row below the first sensor row.
Hope this helps.
Thanks

The flex example is just a starting point for those who have advanced knowledge in css, you may need to add a class to control the flex-direction.
There are some advanced libs that you may use instead such as:

Bootstrap grid:
https://formly.dev/examples/bootstrap-specific/advanced-layout

note: you don't have to import the entire bootstrap style, just the grid one bootstrap-grid.css

Angular flex layout:
https://stackblitz.com/edit/ngx-formly-flex-layout


"TypeError: browser.saveScreen is not a function" if try to run tests on Saucelabs

Environment (please complete the following information):

Node.js version: [18.16.0]
NPM version: [9.5.1]
Browser name and version: [Chrome 114]
Platform name and version: [Windows 10]
WebdriverIO version: [8.11.2]
wdio-image-comparison-service version: [5.0.3]

Config of WebdriverIO + wdio-image-comparison-service
[
        'image-comparison',
        // The options
        {
            // Some options, see the docs for more
            baselineFolder: join(
                process.cwd(),
                './tests/sauceLabsBaseline/'
            ),
            formatImageName: '{tag}-{logName}-{width}x{height}',
            screenshotPath: join(process.cwd(), '.tmp/'),
            savePerInstance: true,
            autoSaveBaseline: true,
            blockOutStatusBar: true,
            blockOutToolBar: true
        },
    ]

Describe the bug
import LoginPage from '../pageobjects/login.page';
import SecurePage from '../pageobjects/secure.page';

describe('My Login application', () => {
    it('should login with valid credentials', async () => {
        await LoginPage.open();

        await LoginPage.login('tomsmith', 'SuperSecretPassword!');
        await expect(SecurePage.flashAlert).toBeExisting();
        await expect(SecurePage.flashAlert).toHaveTextContaining(
            'You logged into a secure area!');

        await browser.saveScreen('examplePaged', {
            /* some options */
        })
    });
});

If I run this test locally - it passes.
To Reproduce
To reproduce bug clone https://github.com/idem7x/wdiosauce.git and follow instructions in README.
Expected behaviour
No error appears, screenshot is added.
Log
[0-0] 2023-06-16T12:33:51.705Z INFO webdriver: [POST] https://ondemand.eu-central-1.saucelabs.com/wd/hub/session/aba2c70a593f499a96c2adebe04b5eb6/execute/sync
[0-0] 2023-06-16T12:33:51.705Z INFO webdriver: DATA {
[0-0]   script: 'sauce:context=TypeError: browser.saveScreen is not a function',
[0-0]   args: []
[0-0] }

This is related to mixed configs. Can you please share all your configs and the order which you import them?

@wbigno
Why do you have this in your sauce config
        [
            ('image-comparison',
            {
                baselineFolder: join(process.cwd(), `visual-regression/${process.env.DOM_BUILD_ENV}/`),
                formatImageName: '{browserName}--{browserVersion}-{tag}',
                screenshotPath: join(process.cwd(), `tmp/${process.env.DOM_BUILD_ENV}/`),
                misMatchTolerance: 0.05,
                savePerInstance: true,
                autoSaveBaseline: true,
                blockOutStatusBar: true,
                blockOutToolBar: true,
                autoElementScroll: false,
                fullPageScrollTimeout: '5000',
                disableCSSAnimation: true
            })
        ],

Meaning , why is it between ()?

Im overriding the services config from my wdio.config. My wdio has image compare, timeline report and dev tools. For sauce I need sauce service, don’t need debtools.. so I have always imported the wdio and then set the services to what I need only for that config. Does that answer your question? Is that not what you would recommend? We have been using this approach for 3 years since v4 of webdriverio. On Dec 30, 2023, at 11:08 PM, Wim Selles @.***> wrote:
@wbigno
Why do you have this in your sauce config
[
('image-comparison',
{
baselineFolder: join(process.cwd(), visual-regression/${process.env.DOM_BUILD_ENV}/),
formatImageName: '{browserName}--{browserVersion}-{tag}',
screenshotPath: join(process.cwd(), tmp/${process.env.DOM_BUILD_ENV}/),
misMatchTolerance: 0.05,
savePerInstance: true,
autoSaveBaseline: true,
blockOutStatusBar: true,
blockOutToolBar: true,
autoElementScroll: false,
fullPageScrollTimeout: '5000',
disableCSSAnimation: true
})
],
Meaning , why is it between ()?
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

I don't mind that you overwrite it, but please use the correct syntax, you have the () which is not needed, change it to this
    services: [
        [
            'sauce',
            {
                sauceConnect: true,
                sauceConnectOpts: {
                    proxyTunnel: true,
                    proxy: 'zproxy.*******.com:9480',
                    pac: 'http://pac.zscaler.net/*******.com/proxy.pac',
                    tunnelCainfo: localCaInfo,
                    logger: async function (message) {
                        console.log(message);
                    }
                },
                setJobNameInBeforeSuite: true
            }
        ],
        [
            'image-comparison',
            {
                baselineFolder: join(process.cwd(), `visual-regression/${process.env.DOM_BUILD_ENV}/`),
                formatImageName: '{browserName}--{browserVersion}-{tag}',
                screenshotPath: join(process.cwd(), `tmp/${process.env.DOM_BUILD_ENV}/`),
                misMatchTolerance: 0.05,
                savePerInstance: true,
                autoSaveBaseline: true,
                blockOutStatusBar: true,
                blockOutToolBar: true,
                autoElementScroll: false,
                fullPageScrollTimeout: '5000',
                disableCSSAnimation: true
            }
        ],
        [TimelineService]
    ],


add json serialization of timestamp/date32/date64

This adds serialization of dates and timestamp. I started with the default Display implementation from chrono. We could add formatter later if that's required.

small conflict

Thanks, @ritchie46 !


Performance Profiling is broken

following the example https://docs.flutter.dev/cookbook/testing/integration/profiling
with the driver as
import 'package:flutter_driver/driver_extension.dart';
import 'package:flutter_template/main_development.dart' as app;

void main() {
  enableFlutterDriverExtension();
  app.main();
}

the app crashes with a bunch of
import 'dart:ui' as ui;
       ^
Failed to stop app

Adding https://docs.flutter.dev/cookbook/testing/integration/introduction#5b-web
import 'package:integration_test/integration_test_driver.dart';

Future<void> main() => integrationDriver();

throws
DriverError: Could not determine URL to connect to application.
Either the VM_SERVICE_URL environment variable should be set, or an explicit URL should be provided to the FlutterDriver.connect() method.

I have another integration test that works fine following the integration tests docs
    testWidgets('tap on the increment floating action button; verify counter',
        (WidgetTester tester) async {
      app.main();

      await tester.pumpAndSettle();
      // Finds the floating action button to tap on.
      final fab = find.byTooltip('Increment');
      // Emulate a tap on the floating action button.
      await tester.tap(fab);
      await tester.pumpAndSettle();
      expect(find.text('1'), findsOneWidget);
    });

It seems like the profiling is broken?
[✓] Flutter (Channel master, 2.6.0-12.0.pre.854, on macOS 11.6 20G165 darwin-x64, locale en)
[!] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
    ✗ cmdline-tools component is missing
      Run `path/to/sdkmanager --install "cmdline-tools;latest"`
      See https://developer.android.com/studio/command-line for more details.
    ✗ Android license status unknown.
      Run `flutter doctor --android-licenses` to accept the SDK licenses.
      See https://flutter.dev/docs/get-started/install/macos#android-setup for more details.
[✓] Xcode - develop for iOS and macOS (Xcode 13.0)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2020.3)
[✓] IntelliJ IDEA Ultimate Edition (version 2018.2.5)
[✓] Connected device (2 available)

Hi @jeremiahlukus, the test file can not contain anything Flutter specific because it is not run on a device/emulator, only the driven app can import such libraries. Please see related issue https://github.com/flutter/flutter/issues/27826.
Closing this as it is not a bug or feature request, if you disagree please write in the comments and I will reopen it.

@mamuseferha yes so my second comment throws
Either the VM_SERVICE_URL environment variable should be set, or an explicit URL should be provided to the FlutterDriver.connect() method.

When having
import 'package:integration_test/integration_test_driver.dart';

Future<void> main() => integrationDriver();

As the driver
 flutter drive --driver=test_driver/integration_test.dart --target=integration_test/counter_profile_test.dart --flavor development --profile

@jeremiahlukus there is an open issue that is similar to this https://github.com/flutter/flutter/issues/15433, please follow up on that issue.


ND4J native slower than JBLAS for small matrices

It seems that using ND4J with native BLAS (e.g. OpenBLAS) is slower than using MTJ/netlib-java with JBLAS, when performing computation with small matrices.
I noticed this initially with a simple feedforward network.
I have boiled the benchmark to a simpler test : just checking matrix multiplication a matrix, for which i vary one dimension.
As dimension grows ND4J / native beats netlib-java, but for small matrice this is not true.
Code is available here :
https://github.com/nsalmon/nnetbench
Similar result on both Linux / Windows
BLAS : windows : OpenBLAS, Linux : not sure
DL4J/ND4J : 0.5.0

You seem to be on to something here.
I've added your benchmarks on a branch of my JMH based suite, in order to make sure we don't run into the usual micro benchmark probles: https://github.com/treo/benchmarking_nd4j/tree/jblas-comparison
And those are my results:
Benchmark                    Mode     Cnt     Score      Error  Units
MatrixMul.jblas_mult_50    sample  672820   291.965 ±    4.749  ns/op
MatrixMul.jblas_mult_100   sample  682634   527.705 ±    4.185  ns/op
MatrixMul.jblas_mult_200   sample  746667   910.276 ±   16.265  ns/op
MatrixMul.jblas_mult_300   sample  481726  1418.399 ±    8.814  ns/op
MatrixMul.jblas_mult_400   sample  750237  1752.942 ±   24.281  ns/op
MatrixMul.jblas_mult_500   sample  610629  2140.361 ±    8.233  ns/op
MatrixMul.jblas_mult_600   sample  515477  2532.502 ±    9.473  ns/op
MatrixMul.jblas_mult_700   sample  443655  2957.285 ±   11.689  ns/op
MatrixMul.jblas_mult_800   sample  390401  3396.029 ±   34.230  ns/op
MatrixMul.jblas_mult_900   sample  698368  3649.836 ±   12.422  ns/op
MatrixMul.jblas_mult_1000  sample  625307  4102.183 ±   12.547  ns/op
MatrixMul.nd4j_mult_50     sample  474364  2162.968 ± 1555.500  ns/op
MatrixMul.nd4j_mult_100    sample  541294  2158.111 ± 1320.987  ns/op
MatrixMul.nd4j_mult_200    sample  661643  2054.711 ±  805.799  ns/op
MatrixMul.nd4j_mult_300    sample  594561  3802.084 ± 2227.630  ns/op
MatrixMul.nd4j_mult_400    sample  537850  2679.986 ±  931.967  ns/op
MatrixMul.nd4j_mult_500    sample  484437  3020.257 ±  897.686  ns/op
MatrixMul.nd4j_mult_600    sample  508129  3477.612 ± 1166.689  ns/op
MatrixMul.nd4j_mult_700    sample  515286  3877.644 ± 1165.312  ns/op
MatrixMul.nd4j_mult_800    sample  499527  4068.887 ± 1193.907  ns/op
MatrixMul.nd4j_mult_900    sample  652886  3978.429 ±   21.687  ns/op
MatrixMul.nd4j_mult_1000   sample  609538  5079.889 ± 2180.397  ns/op

What we are seeing is that we have a huge variance on the timing of nd4j runs. And there seems to be a huge somewhat constant overhead, as the 10x sized version is only about 2.5x slower.

Not sure how jblas handles mult, hadn't got there yet, but mmul uses dup() inside. Can't it be the cause of the issue?

Result array is using createUninitialized: https://github.com/deeplearning4j/nd4j/blob/461f4f1fce039a9f250cfa7449b940e801eca232/nd4j-backends/nd4j-api-parent/nd4j-api/src/main/java/org/nd4j/linalg/api/ndarray/BaseNDArray.java#L2465
I think dup is only used in places like this: https://github.com/deeplearning4j/nd4j/blob/461f4f1fce039a9f250cfa7449b940e801eca232/nd4j-backends/nd4j-api-parent/nd4j-api/src/main/java/org/nd4j/linalg/api/ndarray/BaseNDArray.java#L2590
(Dup is necessary there to avoid overwriting the values we need to actually execute the mmul)

I modified the code slightly as below to run only the smallest configuration with ND4J and did some profiling.
        final int numConfigurations = 1;
        final long numIter = 10000000;
        for (int i = 0; i < numConfigurations; i++) {
            int dim = (1 + i) * 100;
            long timeND4J = testND4J(dim, numIter);
        }

The hot spots look like this.

So, there is a lot of time lost in allocating and deallocating native memory, obviously, but that doesn't account for all the difference. Since the buffers are small and could pretty much all fit in 256 KB memory, I think there's some cache trashing happening here. That would also account for the variance in execution time. The usual solution for that is to reuse buffers when possible, which is typically accomplished by calling the functions that take an output parameter instead of using the ones that return new objects, so let's try to do that...

Ah, doesn't help:
    @Override
    public INDArray mmul(INDArray other, INDArray result) {
        return dup().mmuli(other, result);
    }

But if we use mmuli() directly then everything looks fine:
    public static long testND4J(final int dim,
                                final long numIter) {
        INDArray input = Nd4j.rand(6, 1);
        INDArray w1 = Nd4j.rand(dim, 6);
        INDArray h1 = Nd4j.rand(dim, 1);

        long time = System.currentTimeMillis();
        for (int i = 0; i < numIter; i++) {
            w1.mmuli(input, h1);
        }

        return System.currentTimeMillis() - time;
    }

dim;time MTJ (ms);time ND4J (ms)
100;47;48
200;14;30
300;23;20
400;28;32
500;35;29
600;38;30
700;43;37
800;49;38
900;53;42
1000;61;48
...

re: return dup().mmuli(other, result);
I think a dup() is only necessary here when:
(a) other == result (same object)
(b) other and result share the same underlying DataBuffer
In both cases, we need to dup to avoid overwriting values that are being used in the mmul.
In all other cases, we should be able to remove the dup.

Right, using mmuli with an already allocated target (i.e. doing it more like it is done with jblas), I get the following results (interleaved the jblas results from before for better comparability):
Benchmark                   Mode     Cnt     Score     Error  Units
MatrixMul.jblas_mult_50    sample  672820   291.965 ±    4.749  ns/op
MatrixMul.nd4j_mult_50    sample  712027   515.201 ±  10.701  ns/op

MatrixMul.jblas_mult_100   sample  682634   527.705 ±    4.185  ns/op
MatrixMul.nd4j_mult_100   sample  542749   680.958 ±  15.305  ns/op

MatrixMul.jblas_mult_200   sample  746667   910.276 ±   16.265  ns/op
MatrixMul.nd4j_mult_200   sample  772604   889.677 ±  25.272  ns/op

MatrixMul.jblas_mult_300   sample  481726  1418.399 ±    8.814  ns/op
MatrixMul.nd4j_mult_300   sample  590722  1159.232 ±  15.842  ns/op

MatrixMul.jblas_mult_400   sample  750237  1752.942 ±   24.281  ns/op
MatrixMul.nd4j_mult_400   sample  481340  1444.016 ±  48.033  ns/op

MatrixMul.jblas_mult_500   sample  610629  2140.361 ±    8.233  ns/op
MatrixMul.nd4j_mult_500   sample  417956  1753.220 ±  31.161  ns/op

MatrixMul.jblas_mult_600   sample  515477  2532.502 ±    9.473  ns/op
MatrixMul.nd4j_mult_600   sample  677684  1960.515 ± 116.781  ns/op

MatrixMul.jblas_mult_700   sample  443655  2957.285 ±   11.689  ns/op
MatrixMul.nd4j_mult_700   sample  586416  2266.992 ±  51.814  ns/op

MatrixMul.jblas_mult_800   sample  390401  3396.029 ±   34.230  ns/op
MatrixMul.nd4j_mult_800   sample  534458  2463.071 ±  19.203  ns/op

MatrixMul.jblas_mult_900   sample  698368  3649.836 ±   12.422  ns/op
MatrixMul.nd4j_mult_900   sample  476840  2767.260 ±  34.268  ns/op

MatrixMul.jblas_mult_1000  sample  625307  4102.183 ±   12.547  ns/op
MatrixMul.nd4j_mult_1000  sample  432102  3091.252 ±  33.472  ns/op

We can see that the variability has gone way down and that starting at 200 nd4j starts outperforming jblas.

Interesting. The difference is even larger when we tweak the code for MTJ to this:
    public static long testMTJ(final int dim,
                               final long numIter) {

        Matrix w1 = randomMtjMatrix(dim, 6);
        DenseVector h1 = new DenseVector(dim);

        DenseVector inputVector = new DenseVector(6);
        for (int i = 0; i < 6; i++) {
            inputVector.set(i, 1.0);
        }

        long time = System.currentTimeMillis();
        for (int i = 0; i < numIter; i++) {
            w1.mult(inputVector, h1);
        }

        return System.currentTimeMillis() - time;
    }

Looks like the JVM is able to optimize that code globally better with JBLAS. It's probably inlining stuff and detecting some unnecessary computation given that the input data doesn't change and the output doesn't get used anywhere.

No, that's not it. There's a ~350 ns overhead per call and I have no idea where it's coming from. I've removed everything that could introduce the smallest delay. I've literally emptied out the call to sgemv(), but 99% of the time is still spent calling this function:
JNIEXPORT void JNICALL Java_org_nd4j_nativeblas_Nd4jBlas_sgemv(JNIEnv* env, jclass cls) {
}

Something strange happening with JNI here...

It appears my profiler was playing tricks. Commenting out the call doesn't reduce actual execution time, but I get completely different hot spots:

So it looks like we'll need to reduce the allocation of new Java objects as well if we want to be able to compete with other libraries on small matrices.

Hm, interesting. So it's just the one GemvParameters object being allocation per mmul I think (assuming no copies are necessary). Only idea I have is to move the functionality currently in GemvParameters into BaseLevel2, saving the object allocation there. Could do the same for mmul too; also saves us a little bit on garbage collection at the expense of slightly messier code.
https://github.com/deeplearning4j/nd4j/blob/cf9fe733ace449bc797fb6b25879d8e5aa1683f1/nd4j-backends/nd4j-api-parent/nd4j-api/src/main/java/org/nd4j/linalg/api/blas/impl/BaseLevel2.java#L36

Actually, it's not the allocation itself that takes most of the time, but all the checks in the constructor. So I guess GemvParameters should be cached somehow. Not sure how since it can basically copy the whole array if necessary...

Did this get fixed?

No, didn't get fixed, but it only affects small matrices...

I am using Nd4j to build customized LSTM networks and notice a similar problem. Besides allocateArray and deallocate, huge amount of time is consumed by BaseNDArrray.getDouble

I have two questions:

I checked the code to make sure I didn't invoke getDouble method in my code. How could I remove the "getDouble" hotspot?

I already tried to use in-place operations as much as possible to reduce native array allocation/deallocation. However, as the length of string in each batch varies, there are still many new arrays generated each round. My array size is 50 x [200~500] so I am not sure whether it falls in the "small matrices" scope. How could I reduce allocation time consumption in this case?

Any suggestion is greatly appreciated. Thanks!

Wow.
@harperjiang please file new issue, and bring all you have right there, your hotspots are very bad and very unusual, so i need to investigate them.
Please include pom.xml, neural net configuration & optionally iterators (if you're using custom ones)

Stop.
 Or using ours?

@raver119  I am writing my own implementation directly using ND4J.

You're doing it int suboptimal way then. Working on getting scalars, creating lots of temp arrays etc. Check out how our implementation works inside.


License Issue - License canot be public domain or MIT

This is really a duplicate of the issue filed in the upstream repository here:
https://github.com/fourstix/QwiicSerLCD/issues/27
I filed it again here since this repository is being used for the Arduino IDE library manager.
The high level summary is that this code was derived from LGPL 2.1 sources
LGPL 2.1+ does not allow re-licensing derivative code with a different license with substantially different terms such as public domain or MIT
Below is the duplicate of the issue filed on the upstream repository

While the licensing of LiquidCrystal (or bundled Arduino libraries in general) was not obvious prior to 2015, things got much clearer in 2015 to more clearly state the current and past license for the LiquidCrystal library going back to 2006 was/is LGPL 2.1
This library code is a derivative of LiquidCrystal and LiquidCrystal_I2C both of which were/are licensed LGPL 2.1+
LGPL 2.1+ does not allow re-licensing derivative code with a different license with substantially different terms such as public domain or MIT
The LGPL 2.1+ terms require that this library must also be licensed as LGPL 2.1+ or can use the "+" convert option to convert the license to a later/newer license such as gpl v3
This is from SerLCD.cpp
 * License: This code is public domain but you buy me a beer if you use this and we meet someday (Beerware license).
 *
 * This library is based heavily on the LiquidCrystal_I2C library and the sample code provided with
 * the SparkFun Serial OpenLCD display.  The original LiquidCrystal library by David A. Mellis and
 * modified by Limor Fried and the OpenLCD code by Nathan Seidle at SparkFun.
 *
 * The LiquidCrystal_I2C library was based on the work by DFRobot.
 * (That's the only attribution I found in the code I have. If anyone can provide better information,
 * Plese let me know and I'll be happy to give credit where credit is due.)

I have notes on LiquidCrystal and LiquidCrystal_I2C files that go back more than 10 years.
Unfortunately, some of the web pages & files are no longer available.
Perhaps they can still be found in Internet wayback machine.

Here is a more complete background history of the LiquidCrystal_I2C code.
The key take away is that the LiquidCrystal_I2C library is not an original work by DFRobot.
The DFR code is a derivative of an earlier LiquidCrystal_I2C work by Mario H. which is derivative of LiquidCrystal
The LiquidCrystal_I2C files found out on the net typically have no copyright or licensing notices in them
but they are clearly from the same origin given they are all nearly identical to the much older LiquidCrystal_I2C files by Mario H
which originally were hosted here https://hmario.home.xs4all.nl/arduino/LiquidCrystal_I2C
This web page: http://playground.arduino.cc/Code/LCDi2c
mentions Marios LiquidCrystal_I2C library as being available as of 2009-09-14
Mario's web page also claims his library was released in 2009 and in the V1 zip
image here:
http://www.xs4all.nl/~hmario/arduino/LiquidCrystal_I2C/V1.0/LiquidCrystal_I2C_V1.0.zip
all the files but 1 are dated 2009. And in the info sub directory there is a
file called "readme.txt" which states:
LiquidCrystal_I2C V1.0
The LiquidCrystal_I2C library is a modified version of the standard LiquidCrystal library as found on
the Arduino website.
So given Mario's LiquidCrystal_I2C library predates
DFRobot LiquidCrytal_I2C and DFRobot LiquidCrystal_I2C seems to have started with Mario's code,
DFRobot LiquidCrystal_I2C would be a derivative of Mario's LiquidCrystal_I2C
library and Mario's library is a derivative of the IDE LiquidCrystal library.
While the IDE LiquidCrystal code did not contain copyright notices or license
information in the LiquidCrystal source files themselves, at the point in time when LiquidCrystal_I2C was created,
the IDE LiquidCrystal code was licensed as LGPL 2.1 as indicated in the
license.txt file in the root directory of the arduino.cc IDE repository which
was committed October 11, 2007 and has not changed since.
It can be seen here:
https://github.com/arduino/Arduino/blob/master/license.txt
license.txt was a catchall license for all
Arduino core files and libraries provided by the IDE.
It contains a small note:
===========================
next, the gnu lesser general public license that covers the arduino core
and libraries.

===========================
What is below that tiny comment in the readme file is a LGPL 2.1 license agreement.
So it seems that the intent was that the LiquidCrystal library was released as LGPL 2.1
In 2015 the IDE LiquidCrystal source files were updated to contain a copyright notice and
license within the files.
Copyright (C) 2006-2008 Hans-Christoph Steiner. All rights reserved.
Copyright (c) 2010 Arduino LLC. All right reserved.
[ with a LGPL 2.1+ notice ]
To make it clear that the license for the LiquidCrystal library originally was and still is LGPL 2.1
@bperrybap
New Video support! Upload MP4 and MOV file types. Attach files by dragging & dropping, selecting or pasting them.

Duplicate of this issue.

Duplicate of this issue.

While it is a duplicate, it is an upstream issue, and there is no guarantee that an upstream issue once fixed will be pulled in.
IMO, since this is still an issue in this repository, it needs to remain open until is actually fixed particularly in this case since this repository is being used by the Arduino IDE library manager.
It is important to correct the licensing of the library.
Will you accept a PR that corrects the license file and the code to indicate that the code is licensed LGPL 2.1 ?

While it is a duplicate, it is an upstream issue, and there is no guarantee that an upstream issue once fixed will be pulled in.
IMO, since this is still an issue in this repository, it needs to remain open until is actually fixed particularly in this case since this repository is being used by the Arduino IDE library manager.
It is important to correct the licensing of the library.
Will you accept a PR that corrects the license file and the code to indicate that the code is licensed LGPL 2.1 ?


[Blazor] Tags for custom elements / components should contain a dash

Is your feature request related to a problem? Please describe.
The HTML specification says that new standard HTML elements won't contain a dash. When you create custom elements - in Blazor: components - you might want to ensure forward compatibility, so that your component still works when HTML introduces a new element. To ensure that forward compatibility, your component tag should contain at least one dash. Then you can be sure that your component tag will never collide with any future HTML tag.
In addition, a dash also allows the HTML Parser to notice directly custom elements.
Describe the solution you'd like
Currently the app component in the blazor project template has the tag app in the index.html page. Maybe it should be something like app-root. Other frameworks like Angular use dashes too. Firefox describes custom elements here: https://developer.mozilla.org/en-US/docs/Web/Web_Components/Using_custom_elements, dashes are required.
Also components like SurveyPrompt might require a dash. What if HTML introduces a SurveyPrompt HTML element. If you have a Survey-Prompt tag, the custom Blazor component would still work if HTML would introduce a SurveyPrompt tag.
I'm aware that there's the generated razor.g.cs file that maps the used SurveyPrompt tag to the SurveyPrompt class. But maybe this needs more abstraction. Maybe there should be an attribute for a component that allows you to define a tag.
For example, in Angular a component has a selector/tag, that is completely independent from the class of the component. Maybe we need a similar approach in Blazor. Maybe something like this on the SurveyPrompt component:
@tagName  "Survey-Prompt"

You would use the SurveyPrompt component with the specified tag name like you use it today.
<Survey-Prompt Title="How is Blazor working for you?" />

From the code generation, the @tagName could result in an attribute on the generated class, or in a read-only property set. The generated code where the component is used should look the same.
If something like this for custom tag names exists already, I'm sorry, I didn't find it. I decided to write down my thoughts here, and maybe we can have a discussion. :-)

@thomasclaudiushuber thanks for the suggestion. This is not an area we plan on investing in at this point.


Flysystem v2

This PR contains:

Code formatting updated to PSR12 standard
Added strict typing (and static analysis check using Psalm)
Upgraded Glide to v2
Added support for running testsuite on PHP 8  New Github Actions worfklow file (as TravisCI builds are extremely backlogged)

Hello @ADmad! Thanks for your PR!
I have reserves concerning some aspects of strict typing and void return types. However I'd be happy to merge quickly support for flysystem 2. Would you be able to open a PR containing only flysystem 2 support so that we can discuss types independently?

I had opened a PR for only strict typing changes a long time back, but haven't got any feedback/comment for it #284.
I made the flysystem v2 related changes on top of that as the explicit type errors made the task at hand a lot easier.

@tgalopin I just noticed this comment https://github.com/thephpleague/glide/issues/288#issuecomment-714718628. That's great news. It good to finally have someone who can devote time towards this project :slightly_smiling_face:

Hi @ADmad !
I see that you're still working on this PR: thanks a lot! Given the recent release of Flysystem 2, that's indeed important :) .
However, I would really prefer to merge a PR focusing on Flysystem 2, as the other changes in this PR are unrelated. If that's something you would prefer not to do, I'd completely understand, feel free to tell me and I'll do it in a different PR as soon as I have time.

@tgalopin Can you share what exactly are your reservations regarding strict typing? I stated earlier why I chose to do flysystem v2 related changes on top of the strict typing changes. I would prefer not undoing good work without understanding why they are undesirable.

My opinion on the subject is the following:
Typing in PHP is great way to describe a library API: it enforces the following of this API when implementing interfaces / inheriting classes, it helps IDEs understand how the code should be used and it helps developers get started quicker. I like typing a lot as an extension of phpdoc with several nice features added and a more native feel.
However, strict typing introduces more problems than it solves in my opinion. It creates the need for a check to ensure the strict typing declaration is there (thus creating an obstacle to contribution), it enforces a strict usage of casting that PHP users are not used to and it creates maintenance burdens in some specific cases where it's more difficult to fix with strict typing while keeping BC.
These problems would be fine if the gains were significant. I don't see these gains in any application or library I work on. A testing suite covering well the library is enough IMO. To my knowledge, most of the biggest open-source PHP projects in the world do not use strict typing, but they do have a reliable test suite. That's where we should invest time, not strict typing IMO.
And finally, I also have concerns about specific things like:

Adding : void is unnecessary and forces a BC break if the interface changes. Void is not a normal type, it indicates no type. Adding it as a return typehint only forces child interfaces/classes to not return something, which is not something Liskov substitution principle care about. We don't need to add any : void IMO, people should be able to add a return value in a child if necessary (the @return void is OK though as it tells IDEs that it's the case, which is nice).

I'm not a fan of psalm and tools which are very strict like this for open-source. Two reasons: 1/ it discourages contribution, because it shows a red cross (or several if there are many checks like this) for something that is not as important as tests and 2/ it incentivizes contributors to follow the tool blindly in order to have a green check, even if the tool may be wrong. It's important people stay aware of their code, and I tend to think that these tools incentivize people to think less and follow the tool more.

So to summarize, I think the best plan of action for your work would be:
1/ to create a PR dedicated to adding typehints, without the strict type declaration: I'm definitely in favor of adding them ;
2/ to base this Flysystem 2 PR on that new PR, and implement Flysystem 2 migration (which you already did if I'm not mistaken?)
Would that sound good to you :) ?

I disagree with lot of points you have made (expect for the one regarding void return type) but I am not interested in having a debate over them :slightly_smiling_face:.
The most surprising thing to me is that you don't see value in using static analytics tools like psalm. While they can generate false positives at times, they all allowing configuration the strictness level, so one can adjust them as per their needs/sensibilities.
I have removed the strict type declarations and the void return types. I have kept the use of psalm. It's configured to a reasonably low level 4, which should avoid any deterrent to contribution or having people blindly chase the green check mark. If even that is unacceptable to you, you can easily remove it.
You requested making a separate PR with typing changes first but I am being lazy here and haven't done so. The changes related to typing and those related to flysystem v2 upgrades have been done in separate commits so it should still be easy enough to review them separately.
P.S. You might probably already know that Travis has effectively ended support for free usage even for OSS. The builds are severely backed logged on there and they free credits/time they have provided will get exhausted soon.
Hence I had added a workflow file to run the testsuite using Github Actions. You need to check why Github Actions aren't running for this repo. They run fine on my fork https://github.com/ADmad/glide/actions?query=workflow%3Aci (The test failure is just because I don't have Scrutninizer setup).

@ADmad
It's hard to review changes as PR contains all sorts of other things besides Flysystem 2 (editorconfig, GitHub workflows, strict types).
That said I'm not opposed to these

@piotr-cz Did you read my previous comments and/or see the individual commits?
"The changes related to typing and those related to flysystem v2 upgrades have been done in separate commits so it should still be easy enough to review them separately."
This commit only changes related to type hints https://github.com/thephpleague/glide/pull/291/commits/96513dd540c759e4ce6755fbbcbd8b4c0f63c3c3
This commit only has changes related to flysystem v2 https://github.com/thephpleague/glide/pull/291/commits/c15f0f5f70747af0eb082495282b9b69f11e2f78

Given the fact the the original author could no longer devote time to the project and previous contributions never got any feedback, the existing maintainer(s)/reviewer(s) should give more leeway so someone who has actually put time and effort into this.
Anyone who wants separate PRs can do so by splitting my commits. Unless there are actual code adjustments required, I can no longer spend more time on this.

@ADmad OK, thanks

@ADmad don't worry, having multiple commits is definitely fine for me.
I'll review this asap, thanks a lot for your work and understanding!

Would be great to see flysystem 2 compatibility for this. Thanks for all the work!

@Lustmored If you can review the code and test it locally, that would be super useful!
I'll have a look myself but more eyes are always better

@Lustmored If you can review the code and test it locally, that would be super useful!
I'll have a look myself but more eyes are always better

@tgalopin I am already using it in production without any issues, so can confirm it works. Will review PR today :+1:

@tgalopin I am already using it in production without any issues, so can confirm it works. Will review PR today :+1:

Haha fair enough

Haha fair enough

A question I'm looking to answer (not sure yet) is should we release a new major of Glide for Flysystem 2 or should we try to accept both as inputs?
Depending on the answer to this question, this PR may need to be splitted in different PRs.
My (not strong) opinion is to try to see if we can support both Flysystem 1 & 2 in the current Glide version. It would ease adoption and would allow people to migrate smoothly. Then after than, we could release Glide 2 with types, and perhaps additional things (I have some in mind as well).
WDYT?

A question I'm looking to answer (not sure yet) is should we release a new major of Glide for Flysystem 2 or should we try to accept both as inputs?
Depending on the answer to this question, this PR may need to be splitted in different PRs.
My (not strong) opinion is to try to see if we can support both Flysystem 1 & 2 in the current Glide version. It would ease adoption and would allow people to migrate smoothly. Then after than, we could release Glide 2 with types, and perhaps additional things (I have some in mind as well).
WDYT?

I think supporting both versions should be simple enough with a single wrapper class or some kind of backwards layer in case Flysystem 1 objects are used in Server constructor. I am unsure if it's worth the effort though.
But that would also mean dropping most of this PR (most of the changes are adding types which I understand you'd prefer to postpone to Glide 2?). I can provide simple implementation idea in a separate PR this week if you'd like to explore options.

I think supporting both versions should be simple enough with a single wrapper class or some kind of backwards layer in case Flysystem 1 objects are used in Server constructor. I am unsure if it's worth the effort though.
But that would also mean dropping most of this PR (most of the changes are adding types which I understand you'd prefer to postpone to Glide 2?). I can provide simple implementation idea in a separate PR this week if you'd like to explore options.

I agree, it does feel like supporting both Flysystem 1 and 2 is feasible. That's what I wanted to investigate. If you want to have a look, go for it, that would be super useful!

most of the changes are adding types which I understand you'd prefer to postpone to Glide 2

It's not even a question of preference: if we choose not to upgrade to Glide 2, we can't add typehints as they break BC. Integrating types at the same time as Flysystem 2 support will depend on the difficulty of implementing Flysystem 1 + 2 support in Glide 1.

I agree, it does feel like supporting both Flysystem 1 and 2 is feasible. That's what I wanted to investigate. If you want to have a look, go for it, that would be super useful!

most of the changes are adding types which I understand you'd prefer to postpone to Glide 2

It's not even a question of preference: if we choose not to upgrade to Glide 2, we can't add typehints as they break BC. Integrating types at the same time as Flysystem 2 support will depend on the difficulty of implementing Flysystem 1 + 2 support in Glide 1.

I agree, it does feel like supporting both Flysystem 1 and 2 is feasible. That's what I wanted to investigate. If you want to have a look, go for it, that would be super useful!

All right. I need a few days and will get back with PR :+1:

I agree, it does feel like supporting both Flysystem 1 and 2 is feasible. That's what I wanted to investigate. If you want to have a look, go for it, that would be super useful!

All right. I need a few days and will get back with PR :+1:

There are interfaces/classes which already use interfaces/classes for flysystem v1 for typing in method signature. For e.g. ResponseFactoryInterface::create().
https://github.com/thephpleague/glide/blob/ae5e26700573cb678919d28e425a8b87bc71c546/src/Responses/ResponseFactoryInterface.php#L15
So in order to support both flysystem v1 and v2 you would have to remove the type for the argument which would be a backwards compatibility break and hence warrant a major version bump for Glide anyway.

There are interfaces/classes which already use interfaces/classes for flysystem v1 for typing in method signature. For e.g. ResponseFactoryInterface::create().
https://github.com/thephpleague/glide/blob/ae5e26700573cb678919d28e425a8b87bc71c546/src/Responses/ResponseFactoryInterface.php#L15
So in order to support both flysystem v1 and v2 you would have to remove the type for the argument which would be a backwards compatibility break and hence warrant a major version bump for Glide anyway.

I have just skimmed over code base and @ADmad is right about signature changes.
I see only one way to preserve signatures and be v1 and v2 compliant and this is instead of creating backwards compatibility wrapper, create forwards compatibility one - class decorating v2 FilesystemOperator with signature compatible with v1 FilesystemInterface. Let's call it FilesystemOperatorDriver for now (I know, name is bad).
That would enforce v2 users to wrap initialization code with this class, for example:
$server = ServerFactory::create(
    [
        'cache' => new FilesystemOperatorDriver($cacheStorage),
        'source' => new FilesystemOperatorDriver($publicStorage),
    ]
);

We could also write FilesystemDriver for v1 that'd take FilesystemOperator as only argument and work around that (implement all AdapterInterface method). That'd be more logical way of doing it, but probably require a bit more work and probably end user code would get uglier.
Anyways - it shouldn't be difficult to implement either way (after all v1 and v2 are doing the same things with different philosophies under the hood and different interfaces). But I'd love to hear opinions or other ideas :)

I have just skimmed over code base and @ADmad is right about signature changes.
I see only one way to preserve signatures and be v1 and v2 compliant and this is instead of creating backwards compatibility wrapper, create forwards compatibility one - class decorating v2 FilesystemOperator with signature compatible with v1 FilesystemInterface. Let's call it FilesystemOperatorDriver for now (I know, name is bad).
That would enforce v2 users to wrap initialization code with this class, for example:
$server = ServerFactory::create(
    [
        'cache' => new FilesystemOperatorDriver($cacheStorage),
        'source' => new FilesystemOperatorDriver($publicStorage),
    ]
);

We could also write FilesystemDriver for v1 that'd take FilesystemOperator as only argument and work around that (implement all AdapterInterface method). That'd be more logical way of doing it, but probably require a bit more work and probably end user code would get uglier.
Anyways - it shouldn't be difficult to implement either way (after all v1 and v2 are doing the same things with different philosophies under the hood and different interfaces). But I'd love to hear opinions or other ideas :)

TBH honest I don't think ResponseFactories are much of a problem for many users. What matters to most users is that ServerFactory can accept both Flysystem 1 or 2 natively. We don't need anything there I think, as we can detect which object is provided. We can even detect the Flysystem version to instanciate when a string is passed.
Regarding the interfaces themselves, I would be completely fine to introduce new interfaces to tackle this problem. For instance:
interface Flysystem2ResponseFactoryInterface
{
    public function createFlysystem2(FilesystemOperator $cache, $path);
}

(naming can change)
And have response factories could implement both interfaces:
class PsrResponseFactory implements ResponseFactoryInterface, Flysystem2ResponseFactoryInterface

This is Backward Compatible and allow users of response factories to have essentially nothing to do to migrate to Flysystem 2. It allows for a much smoother transition between Flysystem 1 and 2 for Glide users, as they don't have to upgrade both Flysystem and Glide at the same time (prone to errors).

TBH honest I don't think ResponseFactories are much of a problem for many users. What matters to most users is that ServerFactory can accept both Flysystem 1 or 2 natively. We don't need anything there I think, as we can detect which object is provided. We can even detect the Flysystem version to instanciate when a string is passed.
Regarding the interfaces themselves, I would be completely fine to introduce new interfaces to tackle this problem. For instance:
interface Flysystem2ResponseFactoryInterface
{
    public function createFlysystem2(FilesystemOperator $cache, $path);
}

(naming can change)
And have response factories could implement both interfaces:
class PsrResponseFactory implements ResponseFactoryInterface, Flysystem2ResponseFactoryInterface

This is Backward Compatible and allow users of response factories to have essentially nothing to do to migrate to Flysystem 2. It allows for a much smoother transition between Flysystem 1 and 2 for Glide users, as they don't have to upgrade both Flysystem and Glide at the same time (prone to errors).

I like the way you think about it. As promised I will pursue that goal and come back with a PR :+1:

I like the way you think about it. As promised I will pursue that goal and come back with a PR :+1:

It allows for a much smoother transition between Flysystem 1 and 2 for Glide users, as they don't have to upgrade both Flysystem and Glide at the same time (prone to errors).

@tgalopin I fail to see the potential problem(s). Anyone using Flysystem 1 would continue using Glide 1 and anyone who wants to use Flysystem 2 would upgrade to Glide 2. As @Lustmored stated he's already using my branch without issues.
Trying to support both Flysystem 1 and 2 together will in fact cause problems like the one mentioned by @Lustmored in #300.

One important thing I might have missed some place is exception handling - Flysystem v2 changed a lot and now throws exceptions instead of returning false. I've tried to adapt to it, but probably missed a place or two.

Bumping the major version of a lib doesn't necessarily mean it has to include new features too. Version numbers are free, you can bump the version to 3.0 whenever you have new features ready, which you had planned for v2.

It allows for a much smoother transition between Flysystem 1 and 2 for Glide users, as they don't have to upgrade both Flysystem and Glide at the same time (prone to errors).

@tgalopin I fail to see the potential problem(s). Anyone using Flysystem 1 would continue using Glide 1 and anyone who wants to use Flysystem 2 would upgrade to Glide 2. As @Lustmored stated he's already using my branch without issues.
Trying to support both Flysystem 1 and 2 together will in fact cause problems like the one mentioned by @Lustmored in #300.

One important thing I might have missed some place is exception handling - Flysystem v2 changed a lot and now throws exceptions instead of returning false. I've tried to adapt to it, but probably missed a place or two.

Bumping the major version of a lib doesn't necessarily mean it has to include new features too. Version numbers are free, you can bump the version to 3.0 whenever you have new features ready, which you had planned for v2.

Anyone using Flysystem 1 would continue using Glide 1 and anyone who wants to use Flysystem 2 would upgrade to Glide 2

That's kind of my point :) . Introducing multiple BC breaks at the same time (for instance types + Flysystem 2) would force users to migrate both Glide and Flysystem at the same time, which is always more difficult than upgrading package by package. I'm not saying we shouldn't do it, I'm just saying I much prefer to spend a bit more time now trying to think about the upgrade experience than delegate the potential struggle to users.
I'll have to look for myself how difficult it is to actually make Glide 1 compatible with Flysystem 2. Perhaps we should create a very simple 2.0 with only Flysystem 2.0 support (to ease upgrade) and introduce other BC breaks in a 3.0.

Anyone using Flysystem 1 would continue using Glide 1 and anyone who wants to use Flysystem 2 would upgrade to Glide 2

That's kind of my point :) . Introducing multiple BC breaks at the same time (for instance types + Flysystem 2) would force users to migrate both Glide and Flysystem at the same time, which is always more difficult than upgrading package by package. I'm not saying we shouldn't do it, I'm just saying I much prefer to spend a bit more time now trying to think about the upgrade experience than delegate the potential struggle to users.
I'll have to look for myself how difficult it is to actually make Glide 1 compatible with Flysystem 2. Perhaps we should create a very simple 2.0 with only Flysystem 2.0 support (to ease upgrade) and introduce other BC breaks in a 3.0.

Given your comments and how long this PR has been open for, it seems to me that you have already made up your mind about not merging this PR in its current form.
So I'll just close it and stop bugging you. Thank your for your time.
@Lustmored I won't be deleting my PR branch on my fork and will keep it alive for a sufficient amount of time even after there's an official Flysystem v2 compatible release, so that you can transition without major problems.

Given your comments and how long this PR has been open for, it seems to me that you have already made up your mind about not merging this PR in its current form.
So I'll just close it and stop bugging you. Thank your for your time.
@Lustmored I won't be deleting my PR branch on my fork and will keep it alive for a sufficient amount of time even after there's an official Flysystem v2 compatible release, so that you can transition without major problems.

@ADmad Thanks :+1:

@ADmad Thanks :+1:

@ADmad I understand. I hope it doesn't frustrate you too much. This PR contained indeed too many subjective decisions which have an impact on the project for me to merge it quickly. Generally speaking, I think it's much easier if we focus PRs on specific topics. I'd be happy to discuss them beforehand though, to avoid spending time on it.
Thanks a lot for your help anyway, and please don't hesitate to propose other ideas if you have some for Glide 2/3 !

@ADmad I understand. I hope it doesn't frustrate you too much. This PR contained indeed too many subjective decisions which have an impact on the project for me to merge it quickly. Generally speaking, I think it's much easier if we focus PRs on specific topics. I'd be happy to discuss them beforehand though, to avoid spending time on it.
Thanks a lot for your help anyway, and please don't hesitate to propose other ideas if you have some for Glide 2/3 !


Making iree-util-strip-and-splat-constants use byte_patterns.

Had to remove the path someone added for arith.constant as that can't take byte patterns (and it wasn't the intent of the pass). If someone wants that they can hoist into globals first before running this pass with --iree-util-outline-constants.
A common usage of this would be to take a giant mlir file and run it through iree-opt with
--iree-util-outline-constants --iree-util-strip-and-splat-constants to outline any arith.constant values into globals and then strip them.
Note that the final VMFB will contain the same number of bytes as the original unstripped file.

Thank you!


Support for Xcode 15 accessibility audits

Looks like Xcode 15 supports triggering accessibility audits from XCUI tests, it would be cool to support running an accessibility audit for each of your snapshots.

Good idea! I looked into this a bit and performAccessibilityAudit can be passed a block to handle the resulting issue, however the issue is a XCUIAccessibilityAuditIssue and not a XCTIssue. This makes it tricky to record the issue outside of the test case running when we call performAccessibilityAudit. I think this will require a bit of a refactor to get the app put into a testable state within each test function, rather than the current structure of generating all the images before we create the test functions

This is now implemented in https://github.com/EmergeTools/SnapshotPreviews-iOS/pull/42 thanks for suggesting!


Add chDB integration  doc

Closes https://github.com/chdb-io/chdb/issues/15

:books: Documentation preview :books:: https://jupysql--857.org.readthedocs.build/en/857/

Docs build is failing @nmreadelf

Docs build is failing @nmreadelf

thx, i fixed

the documentation build is failing
you need to add your doc here: https://github.com/ploomber/jupysql/blob/master/doc/_toc.yml
then, you'll need to add chdb here:
https://github.com/ploomber/jupysql/blob/master/doc/environment.yml https://github.com/ploomber/jupysql/blob/master/doc/environment.lock.yml

thx, fixed

awesome, thanks! @nmreadelf feel free to come back to add new stuff to the tutorial. chDB is an exciting project!


Pod template labels in replication controller should be automatically populated

It doesn't make sense to define pod template with a set of labels that is strict subset of replicaSelector (or at least I can't find an use-case). For that reason I think that the labels in pod template part of replication controller should be optional and user should care about them only if he wants to define a superset of what replicaSelector already defines.

I've heard the same comments from a client.
I think that it'd be more intuitive if we always specify pod labels in the template (so that we can copy&paste pod definition), and replicaSelector is optional (by default pod labels).

It would be reasonable for the selector to be a subset.
I'm fine with intelligent defaulting but we need to be careful not to conflate the 2 concepts, which I think would increase the amount of confusion.

Setting RC selector from pod template labels SGTM. Any concerns @smarterclayton? It wouldn't have to work with separate templates, though it would be desirable if it did.
It would be nice if we could add preconditions based on other objects (resource version of pod template in this case) to etcd transactions.

I think we agreed in another issue that defaulting the selector from the pod template labels is reasonable.  No argument from me.  One problem with pod templates is that we lose the strict relationship between selector and template, so if you change the template the RC needs to behave intelligently (I refuse to create more pods because the template selector doesn't match mine, etc)

We should do this, and we should default replicas to 1. We could also consider defaulting replication controller labels from pod template labels. That way, essentially the only thing that would need to be specified for a singleton RC would be the pod template.

I don't think we can default replicas to 1, since we can't tell the difference between an explicit zero and a default zero, and I think there are lots of reasonable cases where you want to set replicas == 0 on create.  (esp. in the context of socket activation)

Responded on the PR: We can distinguish default from 0 if we make it a pointer.


After install, "CoCo format" item colors all default to grey

After installing the extension and restarting VS (2017), all of the "CoCo format" colors defined in Tools / Options / Environment / Fonts and Colors are set to "default". I'd suggest having them initially default to some set color scheme like in the example screen shots.
(An initial peek at the code didn't reveal any pre-set defaults, so I assume this isn't simply a bug, but by all means correct me if I'm wrong!)

No, you are right it's not a bug. I think it will be very strange when you install the new extension and it applies immediately a some pre-set colors to a code.
However, it's only my opinion, so if someone else agree with you maybe I'll set the default values

I can see your point. Maybe a "Use Defaults" option could be added to the custom dialog (Issue #3), as I don't know of a clean way to pop up a "Would you like to use default colors?" dialog upon extension install.
Perhaps make a note in the readme that the colors don't have defaults, so that users don't think the extension isn't working, in case they expected to see the screen shot results on their system right away? It looks like a good extension, and it would be a shame for people to uninstall it thinking it was broken, simply because they didn't know they had to set the colors.
Thanks!


Module naming needs to be consistent in integrated-examples

Module naming needs to be consistent in integrated-examples, you can see the below picture.

I will finish it.


@conda-forge-admin,please add bot automerge

@conda-forge-admin,please add bot automerge

Hi! This is the friendly automated conda-forge-webservice.
I just wanted to let you know that I added bot automerge in conda-forge/datatable-feedstock#5.


Create separate changelog for v2.1

Fixes #2720
Checklist

[x] Changes manually tested
[ ] Automated Tests added/updated
[ ] Documentation added/updated
[x] CLA Signed: DataStax CLA

Checks got messed up when I pointed to wrong branch. Closing, will retry