Consider removing the 'Experimental' tag

What
Consider removing the 'Experimental' tag.
Definitions of the 'Experimental' tag available to users

Most new components and patterns will be published as experimental at first. The Design System team will remove the experimental status once research proves the component or pattern meets user needs. The research must be from different services and with different types of users.

Components and patterns which are not proven usable can be published as experimental. But they must be clearly based on relevant user research from other organisations and best practice, and meet the other criteria.

Why
We don't seem to have a complete, reliable source on the origin of the tag that's written down.
It's also not clear what user research criteria a component/pattern/guidance must prove in order to meet user needs. This has meant that some component/pattern/guidance have been tagged as 'Experimental' for a number of years.
Work on visually differentiating WCAG 2.2 guidance seems to overlap with space on the page, colour of the tag and the need for a strategy around when it is removed.
Comments seen in previously conducted team User Research

Going back to your earlier question [about the 'Experimental tag'], it would be quite nice to be able to understand what that actually means. So that links to an explanation about what is an experimental feature.

I get [experimental]. But like, it's still more research in more depth than I've ever seen on a tab on the internet to me...

Experimental - Not ready yet. error potential

I would be more inclined to feedback on experimental patterns than standard ones P32 ... Experimental: you can use it but it might not be exactly what you are looking for P32 ... How do you keep up to date with an experimental pattern? P32

experimental seems vague, right? It feels like- It's done or it's not done. P55

That’s interesting because I would have to know that it’s experimental, so I would have to do my own research first. But if that was the case I would want to see that they were recording their evidence and sharing it back to the community. I think people should be experimenting and stuff but… you also want things to be consistent. I think with regards to patterns and assessments to the things that are people not using them and not feeding them back into the community.  So, if you are going to do this, let other people know because they can also learn from it, it’s just communication across the community. P56

with an experimental tag, solves a problem that we know and understand, we want people to start using it and see if we’re correct in our assumptions

Conversations that have happened while team members are on support
"one support interaction where a user wanted to know if they could use our bank details pattern or "if it was still experimental". The subtext there is that they presumed:

experimental meant that it wasn't usable in services
there was a process for it not being experimental anymore

I explained this to them and I think they got a bit confused about what experimental meant for their service and their service assessment."
"I think a question is, how should teams treat Experimental components or patterns differently to non Experimental? Some suggestions, I have no strong opinion on them:

a team shouldn't be required to use them in a service assessment
a team should do their own research specifically on this component/pattern

came up on support today, whether you need to start with an experimental pattern in order to pass assessment"
Resources to investigate the issue

A google sheet of all the experimental components/patterns/guidance and when they were labelled as 'Experimental'

Who needs to work on this
Charlotte, Volunteers
Who needs to review this
The whole team
Done when

[x] Find references to the 'Experimental' tag in user research we have already run
[ ] Determine why we have made the decision to tag whole things as 'Experimental' or not experimental in components/patterns/guidance released in the last 3 years
[x] Ask the GOV.UK Design System working group for their thoughts on the parameters of the 'Experimental' tag
[x] Log all 'Experimental' components/patterns/guidance and when they were first published, including anything that has had a label removed
[x] Open a pull request to remove the 'Experimental' tag
[ ] Decide whether to merge the pull request based on team confidence and working group feedback
[ ] If the pull request gets merged, draft a proposal for the working group to review on process tagging which I will cover in a new sprint goal

This work seems to be similar in nature to this previous epic Make "Experimental" tag more understandable or remove https://github.com/alphagov/govuk-design-system/issues/1093 December 2019

FWIW, I'd remove it.
As people have said, it causes confusion.
Plus it's not really a useful label on its own terms. Being experimental or non-experimental isn't a binary thing: you can have a high degree of confidence that something will work in one context, but not another.
The important thing is to be transparent about the contexts in which the pattern has been tried, and any known limitations. The 'Research' section already does that: having an 'Experimental' tag just takes attention away from the nuance.

We are confident that there has never been an instance where an experimental tag has been removed from an experimental entity in the catalogue.
The team has searched closed and open pull requests and the website commit history and they suggest that we have only ever added the experimental tag, never removed it.

We do have another tagged status: Being worked on that is currently applied to the Task list pages pattern because a cross-government group is collaborating on work to update this pattern and build it as a component.

💯 agree, it should be dropped. Far better to note any concerns about aspects of the component or pattern in context within the page.

I’d like to see https://github.com/alphagov/govuk-design-system/issues/2580 done and published before we remove the tag.

Working group thoughts and suggestions on the definition and purpose of the tag

Knee-jerk response: It denotes a component or pattern that has yet to be tested with users or is currently undergoing said testing. This means the component or pattern should be thoroughly tested in our own work before releasing it in the wild and we should feed back our findings to better inform the Design System team and others regarding its use or inclusion in the design system. (Something like that anyway! :blush:)

For me, ‘Experimental’ means the pattern or component hasn’t met a certain threshold of acceptability. But what is that threshold? Is it three services? Is it at least one service in three departments? Etc.
What are the acceptance criteria?
The difficulty, as I see it, is that one service could have rigorously researched a component or pattern, but it would still be experimental for every other service.
If a pattern or component is accepted into the design system, do we need to give it a status (e.g., ‘Experimental’) rather than guidance? Can we simply rely on a service team to make a qualitative assessment of the pattern or component’s validity?

Bit meta, but was the Experimental tag added as an experiment?

I think it was used as a bit of a hedge...?  Have there been any components or patterns which’ve ever had the "experimental" tag removed? I can't think of any...

@frankieroberto we did an investigation and there doesn't seem to be any that have been removed.
I think the conclusions I'm coming to are:

although there seems to be a broad understanding that the tag suggests that more user research is needed, there seems to be discrepancy in the overall definition, purpose of the tag and how the tagged component, pattern or guidance should be used as a result.
when it has been used in the Images | Styles and Resources and tools | Community pages it maybe has a different set of criteria for what 'Experimental' means in these places or at least a different of what the user research looks like
components and patterns where we have updated the
Research on this component
 sections, for example, details, select etc. should have probably been relabelled as 'Experimental'.


unbound: add option to disable ptr records on Overrides/Aliases

Problem:
I don´t always need/want PTR Records for Domain Overrides.
Solution:
Add functionality to disable PTR Records for individual Overrides or Aliases.
Defaults to yes. So existing Overrides don’t get broken.

It's probably better to add a type (A or AAAA without PTR), since MX records don't receive PTR records, the option is confusing in that case.

timeout


chore: Update Dockerfile to ubi9/ubi-micro:9.4

Re: https://github.com/kserve/rest-proxy/pull/42#pullrequestreview-2081513650

We should use a specific version to ensure reproducible builds and/or to trace back in time when something last worked how/why what changed when ...

Alternatively, to stay on UBI 8, we could use the "latest" UBI 8 image ubi8/ubi-micro:8.10-7

TODO: Create architectural decision record issue to summarize why we want to use latest images:
Cons of using latest:

MM image builds might fail unexpectedly
hard to debug/trace back when failing changes happened
last working image may not be possible to recreate for a new MM release without any changes
last working image might not be available even if identified
even if builds succeed, there might be unexpected changes in behavior which happens quite often with 3rd party dependencies of Python packages (on a tangent: when we update to Go 1.19 a while ago, the go get CLI tools install commands "succeed" but no longer installed executables, though we would not use ubi8/go-toolset:latest)

Pros of using latest:

keeping specific version tag, we keep outdated code, old vulnerabilities
we don't have to keep updating to newer patched versions of base images when upstream base images get updated frequently

Builder vs final images:

using latest on a final build stage (not builder image) should be fairly "safe", no 3rd party libraries are installed, just files copied from builder image.
however, it's the base of the builder images where most problems/vulnerabilities are coming in/need to be fixed, are getting fixed upstream,
so really, all build stages need to build on a latest upstream (UBI) base image

Finally, we should follow the same strategy for all MM images consistently.

For go-toolset I do agree, no latest tag.
I didn't understand this last part:

so really, all build stages need to build on a latest upstream (UBI) base image

FIPS


SearchAlabamaStateParks returns 404

Describe the bug
The API call to Search Alabama State Parks returns a 404 error.
Original Camply Command (with --debug)
Here is the response from the call to SearchAlabamaStateParks:
2024-03-03 08:36:37,398 - DEBUG - Starting new HTTPS connection (1): alabamardr.usedirect.com:443
2024-03-03 08:36:37,892 - DEBUG - https://alabamardr.usedirect.com:443 "GET /alabamardr/rdr/search/filters HTTP/1.1" 404 0
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://alabamardr.usedirect.com/alabamardr/rdr/search/filters

Looks like Alabama State Parks is no longer using UseDirect - they switched to something else not supported by camply: https://reserve.alapark.com/


Improved x10 state monitoring

The current x10 component only monitors and checks the state of the 16 devices on 1 house code, the state of the other 240 possible devices are not maintained. Heyu is already used to communicate with the x10 controller and heyu has a state engine that retains the state of all x10 addresses. This updates adds the functionality to fetch the state from the heyu state engine.
The heyu state engine can be started manually by entering 'heyu_engine' at the command line.  If the directive "START_ENGINE AUTO" is included in the configuration file, it will be started along with Heyu's other background process when the command 'heyu start' is entered.
I will update the documentation at https://home-assistant.io/components/light.x10/ when the change is committed.
This update does not affect the configuration.yaml file.

Please clean up the commits from this branch.


🐛 [firebase_messaging 14.3.0 ]  Flutter Android Notification Count is Always null

Bug report
Describe the bug
Flutter android notifications count is always coming null for android and it works fine for IOS
Steps to reproduce
Steps to reproduce the behavior:

Sent notifications from postman or from app
Print the logs to check if notification is coming or not. Notifications title, body, data, color, channel id everything is coming as expected but count[Notification count] for android is always null

Note: I know notifications count will not shown on app icon if it's not enable in the setting to show as number rather than dot.    And yes i am testing this on OPPO device with android OS 12.
Expected behavior
Expected behaviour is it should print the notifications count regardless if it's showing on app icon or not as badge as i'm printing logs when app is active
Sample project
Code for fetching notification when app is active

Output when notifications is received

Sending notification from app:
`var data = {
'to' : 'mytoken',
'notification' : {
"title":"Match update",
"body":"Arsenal goal in added time, score is now 3-0" ,
"priority" : "high" ,
"android_channel_id": "Message" ,
"count" : 10 ,
"notification_count" : 12,
"notification_number" : 12,
"number" : 12,
"badge" : 12,
"click_action": "asif",
"color" : "#FF0000"
},
'data' : {
'type' : 'msj' ,
'id' : 'asif1245' ,
}
};
        await http.post(Uri.parse('https://fcm.googleapis.com/fcm/send'),
        body: jsonEncode(data) ,
          headers: {
            'Content-Type': 'application/json; charset=UTF-8',
            'Authorization' : 'key=myserverkey'
          }
        );`

Please link to the public repository URL.
Source code: https://github.com/axiftaj/Flutter-Firebase-Notifications

Additional context
Flutter doctor
Run flutter doctor and paste the output below:

pub get dependecies

@axiftaj
So when the count is shown as null, do you see the notification badge / count for the app when the notification is triggered ? ie, I am trying to know whether the code is showing count as null but you see the count in the app badge ? For example, when a simple sms arrives, we see a count of total number of messages when app is in background, so in same way, do you see the notification count when app is minimized ?
In this package, it mentions that there's no api exists on native Android to show the badge https://pub.dev/packages/flutter_app_badger#android so I am wondering if this is a limitation of native Android or not.

@darshankawar
NO,
When app is active or in background or in terminated state i always received the notifications but count is not showing on app icon, not null not 1,2,3 or anything.
One thing to clarify i have check the above mention scenario when app is active or in background as i can prints the logs and value is alway null
When we send the notifications from firebase and app is terminated or in background firebase messaging sdk always manage things like showing notifications, displaying badge and message count if we sent from backend.
PS: Let say if native android does't support the badge on icon still value cannot be null for notification count if we are sending it from backend.

Thanks for the feedback. Using the repo shared, I do see same behavior as reported.
Keeping this issue open for team's attention and thoughts on expected behavior in this case.
/cc @Lyokone

@darshankawar @Lyokone
Thank you
Looking forward for a quick fix if their is an issue.

@axiftaj You are not using the correct key to send the notification count to Android; please check documentation.
{
   "android": {
      "notification": {
          "notification_count": 23
      }
   }
}

@axiftaj You are not using the correct key to send the notification count to Android; please check documentation.
{
   "android": {
      "notification": {
          "notification_count": 23
      }
   }
}

@Lyokone  @darshankawar
It's very strange to see you guys closed the ticket without solving it.
Solution you share to use android key is not working, i have used it and now all of the fields are coming null, title, body, channel id everything.
PS: I have updated the repo with proposed changes you mention above but now thing is working.
 var data = {
              'to' : 'token',
              'priority' : 'high' ,
              "android": {
                "notification": {
                  "notification_count": 23,
                  'title' : 'Asif' ,
                  'body' : 'Subscribe to my channel' ,
                  'android_channel_id': "Messages" ,
                  "click_action": 'asif',
                  'color' : '#eeeeee' ,
                }
              },
            };

This code was with notification key was working before for title, body and rest of field except notification count.
 var data = {
              'to' : 'token',
              'priority' : 'high' ,
              'notification' : {
                'title' : 'Asif' ,
                'body' : 'Subscribe to my channel' ,
                'android_channel_id': "Messages" ,
                'count' : 10 ,
                'notification_count' : 12,
                'badge' : 12,
                "click_action": 'asif',
                'color' : '#eeeeee' ,
                //  "image": "https://thenounproject.com/api/private/icons/3689664/edit/?backgroundShape=SQUARE&backgroundShapeColor=%23000000&backgroundShapeOpacity=0&exportSize=752&flipX=false&flipY=false&foregroundColor=%23000000&foregroundOpacity=1&imageFormat=png&rotation=0&token=gAAAAABkHsA94b5pKR5LehBItBteXKuyINT-tsnY1GXXWF7BQgfpnw9LuXqcAGuWMSP3O1CN2io4htufEDwNvCL7n8PAi9K_1Q%3D%3D"
            },
              'data' : {
                'type' : 'msj' ,
                'id' : 'asif1245' ,
              }
            };

Sorry I may have closed it too soon.
You say in your message:

PS: I have updated the repo with proposed changes you mention above but now thing is working.

Can I assume you manage to have it working?

Sorry I may have closed it too soon. You say in your message:

PS: I have updated the repo with proposed changes you mention above but now thing is working.

Can I assume you manage to have it working?

Sorry i have updated my comment. Nothing works with the android key, it was working for title, body etc except notification count.
Also i have updated my repo with the propose changes you asked.
Source code: https://github.com/axiftaj/Flutter-Firebase-Notifications

@darshankawar  @Lyokone
Do i assume this ticket is closed as @Lyokone closed it accidentally and i should open a new ticket.

I'll continue investigation on this one sorry.

Hello @Lyokone, i hope you are doing great.  Any update on this ?

Hello @Lyokone @darshankawar , i hope you are doing great. Any update on this ?

@axiftaj
As mentioned here, team will share updates once they complete investigation.

As previously said, sending the message with the right payload works properly:

See below the code snippet for sending the message:
admin
  .messaging()
  .send({
    data: {
      foo: 'bar',
    },
    notification: {
      title: 'A great title',
      body: 'Great content',
    },
    android: {
      notification: {
        notification_count: 23,
      },
    },
    token,
  })
  .then((res) => {
    if (res.failureCount) {
      console.log('Failed', res.results[0].error);
    } else {
      console.log('Success');
    }
  })
  .catch((err) => {
    console.log('Error:', err);
  });

@darshankawar  @Lyokone
I think this ticket need to opened, here is why.
I am sending notifications using dart code with firebase api in my Flutter App. You have mention that i was not sending the correct payload. Now you can check i'm using the same code as you have mention but still shows count null.
I have attached the source code to help with more investigation.
Source code: https://github.com/axiftaj/Flutter-Firebase-Notifications
Dart code for sending notification but you can also check the repository as well.
var data = {
              'to' : 'your token',
              'notification' : {
                'title' : 'Asif' ,
                'body' : 'Subscribe to my channel' ,
            },
              'android': {
                'notification': {
                  'notification_count': 23,
                },
              },
              'data' : {
                'foo' : 'bar' ,
              }
            };

    await http.post(Uri.parse('https://fcm.googleapis.com/fcm/send'),
        body: jsonEncode(data) ,
          headers: {
            'Content-Type': 'application/json; charset=UTF-8',
            'Authorization' : 'key=serverkey'
          }
        ).then((value){
          if (kDebugMode) {
            print(value);
          }
        }).onError((error, stackTrace){
          if (kDebugMode) {
            print(error);
          }
        });

Output:

@axiftaj Coming back to this issue, I see in the repo you shared, the backgroundHandler method isn't a top-level function, as mentioned in the documentation:

In the plugin example, it is at the top-most level:
https://github.com/firebase/flutterfire/blob/a1363b02fdb12602df535f12bf875f85857524a5/packages/firebase_messaging/firebase_messaging/example/lib/main.dart#L47
But in your repo, it is inside main method:
https://github.com/axiftaj/Flutter-Firebase-Notifications/blob/331823ccad38f2e66e204a4da07975270dca3bcc/lib/main.dart#L15
Check if moving it as top level function helps in your case or not.

@axiftaj Coming back to this issue, I see in the repo you shared, the backgroundHandler method isn't a top-level function, as mentioned in the documentation:

In the plugin example, it is at the top-most level:
https://github.com/firebase/flutterfire/blob/a1363b02fdb12602df535f12bf875f85857524a5/packages/firebase_messaging/firebase_messaging/example/lib/main.dart#L47
But in your repo, it is inside main method:
https://github.com/axiftaj/Flutter-Firebase-Notifications/blob/331823ccad38f2e66e204a4da07975270dca3bcc/lib/main.dart#L15
Check if moving it as top level function helps in your case or not.

In my repo code it is a top level static function as mentioned in the documentation. I am not sure why you you said it's not a top level function.
It's same as plugin example, in plugin example top level is also called inside the main functions.
Beside this this top level function is used to show notification when app is in background. But as you can see notification count is still null when app is active.

Reopening this for @Lyokone's thoughts on OP's recent findings.
I tried using the plugin example and the payload shared here: https://github.com/firebase/flutterfire/issues/10659#issuecomment-1497429865 which gave me correct notification count as below:

Reopening this for @Lyokone's thoughts on OP's recent findings. I tried using the plugin example and the payload shared here: #10659 (comment) which gave me correct notification count as below:

I think notification is sent using Node.js SDK , i am sending notification from app using http package with dart code.
Source code: https://github.com/axiftaj/Flutter-Firebase-Notifications

I'm not sure about using the http package to send it from a Flutter app. You might need to open a ticket in the Firebase REST API to have more information or use the Node SDK. The issue is not related to FlutterFire so nothing we can do here.

I'm not sure about using the http package to send it from a Flutter app. You might need to open a ticket in the Firebase REST API to have more information or use the Node SDK. The issue is not related to FlutterFire so nothing we can do here.

@Lyokone  I don' think it's an issue with http package.  Http package is used to send request to firebase server and request is successful as you can clearly see notification title and body is coming, only count is null for android and it's working fine for iOS.
I think only answer firebase team @darshankawar has to closed the ticket without looking into the matter or hearing back from the reported.

@darshankawar  @Lyokone
Hell0 @darshankawar  iI want you to open this ticket.
I have attach the video below to prove that this is not an http error and something is wrong with plugin because it's showing title and boyd but the the count.
Video link: https://youtu.be/G86qZRswKtE
Looking forward for the response.

Hello @darshankawar, you said we will do the follow up on this ticket but it seems to me this ticket is no longer your priority as i haven't hear back from One week.
I think i'll open a new ticket as this one is closed by @Lyokone  though you said we will follow up on this rather than #10800


Documentation: Add docs for configuring hubble metrics with TLS

Fixes: #33066
Document configuring hubble metrics with TLS

/test

@learnitall thanks for the suggestions, they all seemed reasonable to me so I just updated to the suggestions verbatim.

/test

/test


[event] macro-carousel-slide-visible

This can enable features like lazy loading
[Name could change]

Are you going down the IntersectionObserver path? or were you thinking of something else?


AdminUI: Display of the Identity deletion audit log for deleted identities

Readiness checklist

[x] I added/updated unit tests.
[x] I added/updated integration tests.
[x] I ensured that the PR title is good enough for the changelog.
[x] I labeled the PR.

Description

@Dannyps @daniel-almeida-konkconsulting Could you review the agnular part of this, please?

@stamenione I just noticed two things in the UI:

There is a lot of blank space around the address in the audit log view.  That looks really weird.
The heading is "Deletion Process Details". That's misleading, because we only show the audit logs.


fix(package): upgrade signalk-schema and n2k-signalk @signalk

Please merge! This is making development difficult.


feat(view-doc-source): add site detection to be more precise and resilient

make "is franklin site" check more DOM specific
make doc source computation logic more resilient

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

GitHub release
v6.35.0

Your semantic-release bot :package::rocket:


Problem with compression, extracting and compressing again

I found a problem with the resident\system file.
Extracts the Pulse bigendian file to text1 with Ztr Decompress. I compress this text with Ztr decompress tool. Then extract the ztr file with Ztr decompress tool to text2 and compared it with the first text file.
The keys are ok. Texts appear snippets of compressed text along with the text.
Type: byte-compressed fragment. E The uncompressed text below. No repeating or messing up keys.
Is this a problem of compressing using a ztr file already compressed with Ztr decompress?
I can upload the files if I need to.

Pulse ztr to txt
/*$atips_102_t
Classe: Monstros
Ztr-Decompressor using txt and the Pulse ztr and extract again to txt
/*$atips_102_t
M~#
possu sua
própria
classe
apnd habilide
base{d	sneclasse. Qu{d	f desvolv um~#, usm&iai
dm~#
qum9h scaix no
&ributo
d	m~#.44A	ct-u {Color BlizzardBlue}-fund-doZum~r	8a
habilide
dumcri&ur8umclassdif|e, 9apnd7um{Color BlizzardBlue}habilidescdida>. Reveja
cactíica
dseu
m~#
pcri umequip-vcív9 8modoso
*ios!Classe: Monstros
*/

/*$e029
Formal Silk Hat{End}{Many}Formal Silk Hats{End}{Article}a >>Real text value for key e029
/
/$e029h
F�mal Silk H{End}{Many}F�mal Silk Hs{End}{Article}a >> Compacted text from key e029 | A classic hat to bring out your inner gentleman. >> real text value for key e029h.
The compressor is making a mess with very similar keys. Like: e029 and e029h or atips_103_p3 and atips_103_t

Fixed, thanks for the report.


Call processQueue

Hello,
Is possible to call processQueue from controller??
Thanks.

I got it.
Just save the instance from the init method.
var aux;
$scope.dzOptions = {
   ...
   init: function() {
      aux = this;
   }
};
$scope.submitDropzone = function () {
    aux.processQueue();
};

@Hicask Hi, Thanks for suggestion. I have drafted a new release https://github.com/thatisuday/ngDropzone/releases/tag/v1.0.3 for this functionality.
Please read https://github.com/thatisuday/ngDropzone#enqueuing-file-uploads for implementation.
This is the easiest and safest way. Also checkout test.html source code and preview
Let me know if this is working out for you. :)

@Hicask Now v1.0.4 supports all Dropzone methods
https://github.com/thatisuday/ngDropzone/releases/tag/v1.0.4

I see, great I'll use it, thanks


Roll Flutter Engine from 8baab9d01ad2 to f4a978181cc6 (3 revisions)

https://github.com/flutter/engine/compare/8baab9d01ad2...f4a978181cc6
2023-04-12<EMAIL_ADDRESS>Roll buildroot to 48f8487897b7f40e2744c808694535478d06ca47 (flutter/engine#41119)
2023-04-12<EMAIL_ADDRESS>Add docs on how to debug global generators. (flutter/engine#41102)
2023-04-12<EMAIL_ADDRESS>[Impeller] Add a safe way for runtime effects to provide dynamically generated shader metadata (flutter/engine#40992)
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry<EMAIL_ADDRESS>on the revert to ensure that a human
is aware of the problem.
To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Commit queue failed; closing this roll.


Bug with navigating back from page with ZXingScannerView on UWP

When I put the ZXingScannerView to a page on my UWP Xamarin Forms app I get error on navigating back:
System.InvalidOperationException

|ZXingNetMobile.dll | ZXing::Mobile::ZXingScannerControl::_StopScanningAsync_d__73.MoveNext|

I issued nuget package with the changes (unlisted publicly):
Install-Package pt.ZXing.Net.Mobile.Forms -Version 2.4.1

This was manually merged some time ago, thanks!


mount acme folder instead of file

If you use traefik in Docker and use Let's Encrypt, you have to mount the folder containing the acme.json file instead of the file itself.

ping @NicolasGeraud

LGTM

Sorry @NicolasGeraud, need a rebase...
I'll merge it as soon as rebased.


transfer_batch_to_device doesn't work under DP/DDP/DDP2

🐛 Bug
This is discussed under #1756 and I'm opening a separate issue here for visibility.
In the training loop, for DP/DDP/DDP2, we do not move the data to devices ourselves, but instead use the default scatter to transfer data. This results in transfer_batch_to_device not being called.
https://github.com/PyTorchLightning/pytorch-lightning/blob/16a7326e5259a3cdd20a508c34a0f84806d88f8e/pytorch_lightning/trainer/training_loop.py#L736-L737
Expected behavior
Ideally, we want transfer_batch_to_device to work in all settings. If it's not possible at all to override this behavior, at least a run-time warning and/or some warning in the doc should be given.

Ummm... yeah good point. i'm not sure we can add a hook here. Maybe @awaelchli can look into this

@edenafek  @awaelchli did we add a hook for this now?

No it is not there yet. This would require a custom scatter/gather in LightningDataParallel/LightningDistributedDataParallel that the user defines. Here I am not sure what the recommended way is in Lightning. Should the user subclass these classes and init them in configure_ddp hook?

Not sure if the earlier label removal counts towards a new "activity" by the stale bot, so commenting here to indicate that this is not stale and still needs to be addressed.

@awaelchli is this still relevant?

@edenlightning yes. Still not supported for DP AFAIK.

@awaelchli is it possible to support?

Unlikely it can be supported in the near future. The blocker here is torch.nn.DataParallel which just inherently through it's design cannot scatter and gather arbitrary objects. At this time, I don't see a non-hacky solution.
Users can still provide their own DataParallel wrapper. For example, torch-geometric people have to import the custom DP module from here: https://github.com/rusty1s/pytorch_geometric/blob/master/torch_geometric/nn/data_parallel.py and use that in Lightning (they would also have to do this without Lightning).

I am not sure if these belongs here, but it seems related. I can also open another issue if you prefer.
For me, when I use DDP, my (custom) LightningDataModule.transfer_batch_to_device fails because its batch argument is now a tuple that contains not only the batch but also the batch index and the optimizer index. When I don't use an accelerator, the batch argument only contains a batch, as I would expect.
With normal execution on 1 GPU and without accelerator, the _step method in gpu_accelerator.py (line 59) calls args[0] = self.to_device(args[0]), where args contains the batch, its index and the optim index. transfer_batch_to_device is then correctly called with only the batch (and the device) as argument.
However, with DDP, the _step method in ddp_accelerator.py (line 173) calls self.ddp_plugin.on_before_forward(self.trainer.get_model(), *args), where args is again the batch, idx and optim idx. The ddp_plugin passes all arguments to transfer_batch_to_device without doing anything with them. It seems that this could thus maybe be fixed by changing line 173 into self.ddp_plugin.on_before_forward(self.trainer.get_model(), args[0])?
Or is this the thing you are talking about, which cannot be fixed in the near future? Then what would you recommend us to do to solve this? Add a check for which accelerator is used to transfer_batch_to_device and handle the batch accordingly?

@rubencart This seems to be unrelated, and should not be a problem for DDP. This part of the code underwent a lot of changes lately. If you don't mind, would you send us a repro example in a new issue? If you ping me there and it is fixable, I will fix it.

However, with DDP, the _step method in ddp_accelerator.py (line 173) calls self.ddp_plugin.on_before_forward(self.trainer.get_model(), *args), where args is again the batch, idx and optim idx. The ddp_plugin passes all arguments to transfer_batch_to_device without doing anything with them. It seems that this could thus maybe be fixed by changing line 173 into self.ddp_plugin.on_before_forward(self.trainer.get_model(), args[0])?

this has been updated in the recent refactors, so won't be a problem now. you can try master. the new version will be officially released next week I guess.

@rohitgr7 Okay thanks!

@rohitgr7 is issue still in master?

@edenlightning yes it is just with DP since pytorch DataParallel handles the splitting and transferring of batches to the device internally.

@edenlightning Even though this issue was originally reported as a bug, it is really not a bug. We document that DP is not supported for this hook.
As mentioned already https://github.com/PyTorchLightning/pytorch-lightning/issues/2350#issuecomment-781292760
the users can provide their own DP wrapper as they would anyway have to in regular pytorch code. We could add documentation for how to do that in a faq section somewhere? What do you think?

@SeanNaren maybe add to your PR?


[Typescript Error] useTranslation - Type instantiation is excessively deep and possibly infinite

Hi, thank you for the awesome framework, it's so amazing!
🐛 Bug Report
I have two question:

The first one is related to the type error
I get the typescript error Type instantiation is excessively deep and possibly infinite while using useTranslation composable function in a Vue component. I've searched some similar issues but did not find the solution.
I follow the youtube video to setup typescript for i18next: https://www.youtube.com/watch?v=GLIas4DH3Ww
It seems doesn't work properly.

To Reproduce
https://stackblitz.com/edit/github-lup5wg?file=src%2Fcomponents%2FComponentTypeError.ts
Screenshot:

Both useTranslation() and t() has the type error:
import { h, computed } from 'vue';
import { useTranslation } from 'i18next-vue';

export default {
  name: 'ComponentTypeError',
  setup() {
    // Here is the type error
    const { t } = useTranslation();

    // Here is the type error
    const insurance = computed(() => t('insurance'));

    return () => {
      return h('div', `inline => ${insurance.value}`);
    };
  },
};

Expected behavior

Should not cause typescript issues
The autocompletion should work properly

Your Environment

runtime version: node v18(locally), stackbliz(node v18.8)
i18next version: 23.5.1
i18next-vue: 3.0.0
typescript: 5.2.2
os: Mac

Sorry it's my fault I missed the typeof in the following code:
import Resources from './resources';

declare module 'i18next' {
  interface CustomTypeOptions {
    defaultNS: 'translation';
    resources: typeof Resources;
  }
}

Ah, no problem. Glad you figured it out already!


upgrade to hazelcast 3.10.4

fyi, having this running on our test systems for a while now, the memory utilization does not look like it's leaking any more

can you review this for 3.5.4 @tsegismont ?


Select color for button background on Horizontal button stack

I'm trying out the card, which is great, BTW.
one thing I noticed is that when I chose a normal light (dimmer, but no white balance or color control), it appears to use a gray for th background of the button. Is there a way to make this configurable or have it at least default to yellow? Or am I missing something?

I would also love to adjust the color and maybe add some state.


feat: Separate runtimes of SSA functions before inlining

Description
Problem*
Resolves https://github.com/noir-lang/noir/issues/5043

Opcode count of Aztec public functions before => after this change in the inlining strategy
_approve_bridge_and_exit_input_asset_to_L1 on contract Uniswap with size 3788 => 3166
_assert_token_is_same on contract TokenBridge with size 916 => 640
_assert_token_is_same on contract Uniswap with size 1845 => 1546
_borrow on contract Lending with size 10988 => 10152
_call_mint_on_token on contract TokenBridge with size 1427 => 1125
_check_deadline on contract Crowdfunding with size 1015 => 752
_deposit on contract Lending with size 1699 => 1220
_increase_public_balance on contract GasToken with size 1325 => 1103
_increase_public_balance on contract Token with size 1577 => 1201
_increase_public_balance on contract TokenBlacklist with size 1577 => 1201
_reduce_total_supply on contract Token with size 945 => 626
_reduce_total_supply on contract TokenBlacklist with size 945 => 626
_repay on contract Lending with size 6775 => 6284
_withdraw on contract Lending with size 10715 => 9850
add_args_return on contract AvmNestedCallsTest with size 15 => 15
add_args_return on contract AvmTest with size 15 => 15
add_storage_map on contract AvmTest with size 797 => 664
add_to_tally_public on contract EasyPrivateVoting with size 1496 => 1112
add_u128 on contract AvmTest with size 187 => 187
admin on contract Token with size 495 => 348
approve_public_authwit on contract EcdsaAccount with size 977 => 718
approve_public_authwit on contract SchnorrAccount with size 743 => 621
approve_public_authwit on contract SchnorrHardcodedAccount with size 742 => 620
approve_public_authwit on contract SchnorrSingleKeyAccount with size 742 => 620
assert_block_number on contract AppSubscription with size 920 => 669
assert_minter_and_mint on contract Token with size 1434 => 1035
assert_not_expired on contract AppSubscription with size 910 => 658
assert_nullifier_exists on contract AvmTest with size 242 => 156
assert_public_global_vars on contract Test with size 1054 => 679
assert_same on contract AvmNestedCallsTest with size 138 => 138
assert_unsiloed_nullifier_acvm on contract AvmAcvmInteropTest with size 102 => 16
assertion_failure on contract AvmTest with size 166 => 166
avm_to_acvm_call on contract AvmAcvmInteropTest with size 287 => 169
balance_of_public on contract GasToken with size 651 => 612
balance_of_public on contract Token with size 880 => 710
balance_of_public on contract TokenBlacklist with size 880 => 710
borrow_public on contract Lending with size 1193 => 939
broadcast on contract Benchmarking with size 419 => 337
burn_public on contract Token with size 2426 => 1962
burn_public on contract TokenBlacklist with size 3148 => 2468
call_acvm_from_avm on contract AvmAcvmInteropTest with size 574 => 503
call_avm_from_acvm on contract AvmAcvmInteropTest with size 578 => 507
check_balance on contract GasToken with size 856 => 768
check_selector on contract AvmTest with size 463 => 463
claim_public on contract GasToken with size 3963 => 3416
claim_public on contract TokenBridge with size 4176 => 3457
constant_field_acvm on contract AvmAcvmInteropTest with size 13 => 13
constant_field_avm on contract AvmAcvmInteropTest with size 13 => 13
constructor on contract AppSubscription with size 3376 => 1983
constructor on contract Auth with size 1414 => 994
constructor on contract AvmInitializerTest with size 1290 => 908
constructor on contract Claim with size 1700 => 1123
constructor on contract EasyPrivateVoting with size 1454 => 941
constructor on contract FPC with size 1700 => 1123
constructor on contract InclusionProofs with size 1021 => 704
constructor on contract Token with size 2737 => 1961
constructor on contract TokenBlacklist with size 1925 => 1285
constructor on contract TokenBridge with size 1400 => 924
constructor on contract Uniswap with size 1326 => 909
consume_message_from_arbitrary_sender_public on contract Test with size 2203 => 1893
consume_mint_public_message on contract Test with size 2910 => 2600
create_different_nullifier_in_nested_call on contract AvmNestedCallsTest with size 743 => 639
create_l2_to_l1_message_arbitrary_recipient_public on contract Test with size 56 => 12
create_l2_to_l1_message_public on contract Test with size 73 => 29
create_same_nullifier_in_nested_call on contract AvmNestedCallsTest with size 741 => 637
debug_logging on contract AvmTest with size 725 => 402
deposit_public on contract Lending with size 2286 => 1846
dummy_public_call on contract Test with size 354 => 284
emit_nullifier_and_check on contract AvmTest with size 436 => 314
emit_nullifier_public on contract Test with size 47 => 11
emit_unencrypted on contract Test with size 54 => 16
emit_unencrypted_log on contract AvmTest with size 757 => 685
end_vote on contract EasyPrivateVoting with size 514 => 279
exit_to_l1_public on contract TokenBridge with size 2154 => 1773
get_address on contract AvmTest with size 45 => 13
get_args_hash on contract AvmTest with size 18 => 18
get_asset on contract Lending with size 888 => 718
get_assets on contract Lending with size 587 => 411
get_authorized on contract Auth with size 597 => 443
get_authorized_delay on contract Auth with size 702 => 517
get_block_number on contract AvmTest with size 45 => 13
get_chain_id on contract AvmTest with size 45 => 13
get_fee_per_da_gas on contract AvmTest with size 45 => 13
get_fee_per_l2_gas on contract AvmTest with size 45 => 13
get_portal_address_public on contract TokenBridge with size 297 => 145
get_position on contract Lending with size 3908 => 3623
get_price on contract PriceFeed with size 617 => 579
get_public_value on contract StatefulTest with size 636 => 598
get_roles on contract TokenBlacklist with size 989 => 749
get_scheduled_authorized on contract Auth with size 595 => 443
get_sender on contract AvmTest with size 45 => 13
get_shared_immutable_constrained_public on contract DocsExample with size 438 => 406
get_shared_immutable_constrained_public_indirect on contract DocsExample with size 701 => 607
get_shared_immutable_constrained_public_multiple on contract DocsExample with size 136 => 98
get_storage_address on contract AvmTest with size 45 => 13
get_timestamp on contract AvmTest with size 45 => 13
get_token on contract TokenBridge with size 513 => 366
get_transaction_fee on contract AvmTest with size 45 => 13
get_version on contract AvmTest with size 45 => 13
increment_balance on contract Benchmarking with size 1472 => 1264
increment_public_value on contract StatefulTest with size 695 => 449
increment_public_value_no_init_check on contract StatefulTest with size 447 => 351
init on contract Crowdfunding with size 2100 => 1341
init on contract Lending with size 958 => 598
initialize_public_immutable on contract DocsExample with size 433 => 252
initialize_shared_immutable on contract DocsExample with size 433 => 252
is_minter on contract Token with size 838 => 668
is_time_equal on contract Test with size 56 => 18
keccak_hash on contract AvmTest with size 49 => 49
l1_to_l2_msg_exists on contract AvmTest with size 65 => 17
mint_private on contract Token with size 1247 => 790
mint_private on contract TokenBlacklist with size 1495 => 906
mint_public on contract GasToken with size 946 => 813
mint_public on contract Token with size 2048 => 1520
mint_public on contract TokenBlacklist with size 2821 => 2137
modulo2 on contract AvmTest with size 16 => 16
nested_call_to_add on contract AvmNestedCallsTest with size 862 => 756
nested_call_to_add_with_gas on contract AvmNestedCallsTest with size 929 => 799
nested_static_call_to_add on contract AvmNestedCallsTest with size 890 => 784
nested_static_call_to_set_storage on contract AvmNestedCallsTest with size 759 => 665
new_note_hash on contract AvmTest with size 47 => 11
new_nullifier on contract AvmAcvmInteropTest with size 47 => 11
new_nullifier on contract AvmNestedCallsTest with size 47 => 11
new_nullifier on contract AvmTest with size 47 => 11
new_nullifier_acvm on contract AvmAcvmInteropTest with size 47 => 11
note_hash_exists on contract AvmTest with size 65 => 17
nullifier_collision on contract AvmTest with size 82 => 12
nullifier_exists on contract AvmTest with size 103 => 17
on_card_played on contract CardGame with size 2386 => 2151
on_cards_claimed on contract CardGame with size 2176 => 1927
on_game_joined on contract CardGame with size 1972 => 1733
pay_refund on contract FPC with size 2052 => 1685
pay_refund_with_shielded_rebate on contract FPC with size 2138 => 1771
pedersen_hash on contract AvmTest with size 19 => 19
pedersen_hash_with_index on contract AvmTest with size 19 => 19
poseidon2_hash on contract AvmTest with size 1370 => 1370
prepare_fee on contract FPC with size 1643 => 1280
pub_call_public_fn on contract ImportTest with size 710 => 636
pub_entry_point on contract Parent with size 263 => 189
pub_entry_point_twice on contract Parent with size 497 => 339
pub_get_value on contract Child with size 174 => 22
pub_get_value on contract StaticChild with size 409 => 281
pub_illegal_inc_value on contract StaticChild with size 448 => 336
pub_inc_value on contract Child with size 175 => 45
pub_inc_value on contract StaticChild with size 175 => 45
pub_inc_value_internal on contract Child with size 552 => 340
pub_set_value on contract Child with size 108 => 30
pub_set_value on contract StaticChild with size 108 => 30
public_call on contract StaticParent with size 263 => 189
public_constructor on contract StatefulTest with size 1808 => 1399
public_delegate_set_value on contract Delegator with size 724 => 724
public_get_decimals on contract Token with size 557 => 411
public_get_name on contract Token with size 538 => 392
public_get_symbol on contract Token with size 546 => 400
public_get_value_from_child on contract StaticParent with size 759 => 685
public_nested_static_call on contract Parent with size 674 => 561
public_nested_static_call on contract StaticParent with size 1012 => 894
public_set_value on contract DelegatedOn with size 68 => 28
public_static_call on contract Parent with size 328 => 236
public_static_call on contract StaticParent with size 328 => 236
push_nullifier_public on contract InclusionProofs with size 285 => 117
read_storage_immutable on contract AvmInitializerTest with size 286 => 133
read_storage_list on contract AvmTest with size 101 => 62
read_storage_map on contract AvmTest with size 385 => 341
read_storage_single on contract AvmTest with size 66 => 27
register on contract KeyRegistry with size 6385 => 5041
repay_public on contract Lending with size 2141 => 1747
rotate_npk_m on contract KeyRegistry with size 2592 => 1843
send_l2_to_l1_msg on contract AvmTest with size 56 => 12
set_admin on contract Token with size 528 => 257
set_authorized on contract Auth with size 925 => 474
set_authorized_delay on contract Auth with size 679 => 400
set_minter on contract Token with size 865 => 559
set_opcode_big_field on contract AvmTest with size 19 => 19
set_opcode_small_field on contract AvmTest with size 13 => 13
set_opcode_u32 on contract AvmTest with size 13 => 13
set_opcode_u64 on contract AvmTest with size 13 => 13
set_opcode_u8 on contract AvmTest with size 13 => 13
set_portal on contract GasToken with size 434 => 272
set_price on contract PriceFeed with size 401 => 349
set_read_storage_single on contract AvmTest with size 122 => 42
set_storage_list on contract AvmTest with size 76 => 30
set_storage_map on contract AvmTest with size 409 => 355
set_storage_single on contract AvmNestedCallsTest with size 65 => 25
set_storage_single on contract AvmTest with size 65 => 25
set_value_twice_with_nested_first on contract Child with size 803 => 654
set_value_twice_with_nested_last on contract Child with size 791 => 654
set_value_with_two_nested_calls on contract Child with size 1308 => 1063
sha256_hash on contract AvmTest with size 47 => 47
shield on contract Token with size 2368 => 1809
shield on contract TokenBlacklist with size 3102 => 2315
spend_public_authwit on contract DocsExample with size 14 => 14
spend_public_authwit on contract EcdsaAccount with size 1008 => 637
spend_public_authwit on contract SchnorrAccount with size 770 => 541
spend_public_authwit on contract SchnorrHardcodedAccount with size 770 => 540
spend_public_authwit on contract SchnorrSingleKeyAccount with size 770 => 540
spend_public_authwit on contract Uniswap with size 749 => 486
start_game on contract CardGame with size 2518 => 2175
swap_public on contract Uniswap with size 7983 => 6584
test_authwit_send_money on contract AvmAcvmInteropTest with size 803 => 709
test_get_contract_instance on contract AvmTest with size 522 => 410
test_get_contract_instance_raw on contract AvmTest with size 159 => 83
test_nullifier_inclusion_from_public on contract InclusionProofs with size 332 => 121
to_radix_le on contract AvmTest with size 187 => 187
total_supply on contract Token with size 536 => 390
total_supply on contract TokenBlacklist with size 536 => 390
transfer_public on contract Token with size 3071 => 2512
transfer_public on contract TokenBlacklist with size 4491 => 3533
u128_addition_overflow on contract AvmTest with size 772 => 668
u128_from_integer_overflow on contract AvmTest with size 357 => 357
update_accumulator on contract Lending with size 8695 => 8358
update_leader on contract DocsExample with size 430 => 290
update_roles on contract TokenBlacklist with size 2058 => 1252
withdraw_public on contract Lending with size 1190 => 93

Summary*
This PR:

Creates a separate pass for "runtime separation" where we convert any Acir function called from brillig to a brillig function. After that pass, all functions have correct runtime, meaning they'll be converted to the runtime they hold.
Updates the inliner to only consider entry points all brillig functions called from ACIR and all brillig recursive functions, instead of all brillig functions

With this change the inliner can correctly detect which brillig functions must not be inlined. This opens the door to allowing annotating brillig functions with inlining suggestions in the future.
Additional Context
Documentation*
Check one:

[x] No documentation needed.
[ ] Documentation included in this PR.
[ ] [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

[x] I have tested the changes locally.
[x] I have formatted the changes with Prettier and/or cargo fmt on default settings.

I'm wondering if we can avoid the extra pass at all and whether we can avoid the extra passes through the IR that are now within the inlining pass to look for recursive functions and functions that can be inlined.

I think it can be avoided, but then the inlining step becomes footgun land:

In order to find brillig entry points, it has to explore recursively the SSA, instead of just searching for acir functions that call brillig
Recursive functions in the SSA could marked as ACIR but called from brillig. This approach correctly handles this case, but without the runtime separation pass it'll try to unroll the recursive function even in a brillig context, so it'll need to clone the recursive function to brillig.

In general I think it's saner to have a pass that makes the function.runtime express what the function will actually be converted to and reduce the inliner responsibility to just inline functions instead of dealing with context-dependant runtimes. And even if we don't have the pass the inliner will have to do most of the pass work so I think it's cleaner to have it in a separate pass with clear responsibilities.

Recursive functions in the SSA could marked as ACIR but called from brillig. This approach correctly handles this case, but without the runtime separation pass it'll try to unroll the recursive function even in a brillig context, so it'll need to clone the recursive function to brillig. Let me add an e2e test case for this.

Added the e2e test case that fails on nightly and is fixed by this process.

I'm in favour of having this as a separate pass for simplicity and ease of testing. If necessary those two passes can be merged in future.

Possibly the passes that run after unrolling are the ones that have more impact, since unrolling (logically) tends to increase a lot instruction counts

I thought about making this pass more lightweight via collecting called functions using func.values_iter(), but I think it makes it way more brittle, since we could have unused function values there ):

If performance of this pass is a concern I can possibly explore the SSA only once and build a cache of functionId => calledFunctionIds to be reused across the pass. But I'm not sure it'll make much of a difference

Nah, it won't be much of a difference because we do need to reexplore to trim unreachable functions anyway

Yeah, I didn't see the cache making much of a difference either.
Anyway - the main reason I blocked this yesterday was to give me more time to review today not that I didn't think this should be merged. I'll finish the review now

Still no permission to add to merge queue ): @TomAFrench


Header buttons is not updating the output model

When clicking on select-all, I log my output-model, and it's empty. Same goes for Reset, Clear etc

@artdias90 ,
Try to use $timeout.

thanks, forgot to update here. it solved


unexpected literal coercion warning 30081 on half vector types

Trying to compile some of my shaders with slang I'm running into numerous instances of this warning:
warning 30081: implicit conversion from 'int' to 'half' is not recommended

This is caused by expressions like this:
vector<half,2>(0, 0)

This seems like an error. Coercion of 0 should not produce a warning, since 0 can be represented exactly using half floating point.
The same code does not produce any warning when the base type is float or when it's a scalar type:
vector<float,2>(0, 0)
half(0)

Thanks for addressing this so quickly!


C#: embed the pgp keys for mono and dotnet repos instead of using Ubuntu keyserver

Should help with reliability.

Thanks! This branch was deployed to staging. Could you run a smoke test in https://staging.travis-ci.org and link back the results?
I am happy to ignore the false test negatives for now, I'll see to it that the experience improves soon. Sorry about that!

@joepvd I've tested this on staging and it works as expected so I think this is ready :)


Add grouped updates

Our company is migrating to Dependabot from another similar tool (Depfu).
One of the features we miss is grouped updates (when all available updates are merged into 1 PR). Here's how the option is configured for every repository in Depfu's settings (just for example):

Thanks for the feedback @anym0us. We love what the Depfu folks did.
We don't have support for grouped updates yet, so if that's a feature you really need I recommend you stick with Depfu for now. We'll get there on it, but it's just not the way Dependabot currently works.

I'm really interested in this feature to reduce the number of Dependabot PRs. I see that the PR was pinned a month ago. Would it possible to clarify whether this is in the roadmap? Thanks!

We're interested in this feature as it makes it a lot easier to view it all in one PR. Wondering when it is planned for release also, cheers!

We're also interested in this feature. If there are any estimates, it would be much appreciated!

PR simplicity + less notification noise + less Github Actions minutes spent processing dep PRs :)

Beyond reduction of PR noise and build times, grouping would be valuable for libraries that need to be updated together for purposes of compatibility.
For example, updating a single AWS CDK package can lead to test failures due to API compatibilities with other CDK packages. Upgrading them all at once generally mitigates this issue.
Until update grouping is possible, we have to disable CDK auto-updates and apply them manually.

Same for react + react-dom - these always need to be updated in tandem.

I would love to see something like this. It could actually be really useful to have all minor/patch updates grouped in one PR, and separate PRs for major version updates.

Grouping across directories would be amazing. Every time there is a release, we get one PR per directory, and it gets a bit crazy.

would love to see this for jest, babel, react, and storybook packages as they all use lerna and you generally need to update all the packages at once

Would love to see this. Having it grouped by vendor would be amazing, projects like Symfony tag all their packages at the same time so it rains updates, having all those combined into one PR saves us so much more time <3

Definitely would like to see this for babel and similar updates. We were using Renovatebot before, which did this nicely with "babel monorepo" updates.

I really hope this issue is being prioritized.  Since adding Dependabot, I've found it to actually be a productivity killer—the opposite of its intended use.  Instead of individually merging 10-15 PRs every other day, I've gotten in the habit of using it merely as an update notifier and manually updating dependencies myself in a single commit.  Even in this workflow it still causes a lot of spam in our list of pull requests, though.  Trying to look through the closed PR history becomes a real challenge.

That's essentially what we're considering now. Using dependabot as a vuln notifier, but actually manually managing updates or using renovate's self-hosted mode with actions since it has grouping support.

I know this is a bit of a pile on at this point - but the other pain point I'd like to point out is how many CI cycles get wasted due to this. Every PR dependabot sends to a repo triggers CI. For those of us that prefer to group updates (or need to, often because of multiple dependencies needing to update at the same time), this means 50, 100, or even more CI runs that necessary can happen every single week.

I was missing this feature too, so I created a GitHub workflow to automatically combine/group all Dependabot PRs together into a single PR. I figured others could benefit from it too, so I made it available here: https://github.com/hrvey/combine-prs-workflow

Hi,
I was the one who recently reported and disclosed the vulnerability in JUnit 4.
It seems that by doing so I've caused Depenabot to generate over 290,000+ pull requests (and counting).
https://github.com/search?o=desc&q=Bump+junit+from+to+4.13.1+GHSA-269g-pwp5-87pp&s=&type=Issues
There are over 1,508,273+ usages of JUnit 4 according to the GitHub Dependency Graph.
Due to this vulnerability, one of the projects we maintain at Gradle ended up having 101 pull requests generated for it to fix each individual POM.xml file.
https://github.com/gradle/performance-comparisons/pulls?q=is%3Apr+sort%3Aupdated-desc+is%3Aclosed+junit
There is absolutely no way that you can sanely merge 100+ pull requests using the GitHub UI currently. Thus, we were forced to create a seperate PR and close the 101 dependabot generated PRs.
We're not the only ones experiencing this issues, it is reflected on twitter by various others:

https://twitter.com/sormuras/status/1316010270590812160
https://twitter.com/headius/status/1316055330657243142?s=20
https://twitter.com/MarcNuri/status/1316067844602331136?s=20
https://twitter.com/CedricChampeau/status/1315918744036741120?s=20
https://twitter.com/olamy/status/1315908629954916352?s=20

IMHO, if dependabot is going to generate over 10 PRs against a single project for the same dependency, it should consolidate it into one PR with an optional command to explode the PR into individual PRs should the maintainer wish for that instead.
Something like dependabot separate

For anyone looking for a Node alternative to this issue, I created a GitHub action that mimics what is asked here: https://github.com/neverendingqs/gh-action-node-update-deps.
It uses npm-check-updates and hub to create a pull request with updates to all dependencies. It's not as feature-rich as dependabot, but leaving security updates enabled and running this weekly has been sufficient to keep my dependencies updated.

In several repositories (while not in others), dependabot started grouping react and react-dom:
https://github.com/wix/potato-bruce/pull/145
Is the dependabot team experimenting with this? Would be great to have a way to specify npm scopes that update together (@jest, @babel,etc.)

I've written a potential spec here.
https://github.com/dependabot/dependabot-core/issues/2672
I'm not really sure what the status is on this feature. But it would be nice to have these updates in.

We have introduced dependabot into my company's GHE. After some team start to use it, they complains a lot about the PR hells.
The group feature will make our life easier.

I would prefer if grouping could be configured on a per-organization basis:
For example, I get a lot of PRs for @types-dependencies, e.g. @types/node, @types/lodash and so on.
Even in small projects like attranslate, I get quite a lot of @types-PRs: https://github.com/fkirc/attranslate/pulls?q=%40types+
Those @types-dependencies are not super-critical because they do not change any runtime-behavior, therefore it would be easy to merge multiple @types-dependencies in a single PR.
If I could configure a grouping for the @types-organization, then this would be already a huge help to reduce noise and overheads.
Of course, @types is not the only organization that is good for grouping.
For example, @stephanvierkant pointed out that @symfony could be a good grouping-candidate as well:
https://github.com/dependabot/dependabot-core/issues/2265#issuecomment-626533927
Therefore, it seems natural to make organization-grouping configurable instead of hardcoding a specific behavior.

I'd like to see "group patch updates", nested under "group minor updates", nested under "group major updates". I can't imagine most would use the major, but for hobby projects, maybe. Still seems like a simple config, and already plays well with the existing "auto merge" config.

I also agree that updating should be configurable. E.g. all @aws-cdk/*-scoped packages are released at the same time. However, other scoped packages could be less dependant on each other.

Personally what I miss most is a way to group in a single PR the update of a single dependency that is used in multiple package.json (instead of having one PR per package.json).

The biggest pain we have that once you merge one PR you need to rebase (which is crucial to not get broken dependencies tree) others one by one and wait for tests to pass each time.
When you have, let's say, 6 PR from dependabot and your tests take ~10 minutes to merge all PRs you need to wait one hour.
It could help if there would be an auto-merge feature to not struggle with merge manually, but dependabot lost it during the migration to GitHub: https://github.com/dependabot/dependabot-core/issues/1973 (and it's not an option to workaround this with GitHub Actions since we've got our own CI infrastructure)
dependabot's support for private registries is currently (https://github.com/github/roadmap/issues/67) vendor-locked to Github Packages, which is one of the main reasons pushed our company to migrate to GitHub packages. But since dependabot has no auto-merge and no grouping it's now faster and easier to bump dependencies locally than struggle with PRs rebasing hustle. This makes dependabot useless 😢

One thing to add is that renovate can even detect packages that belong together that are not in the same repository or follow any organization scoping.
See https://github.com/dungeon-revealer/dungeon-revealer/pull/889 as an example.

For information: https://github.com/github/roadmap/issues/133

For information: https://github.com/github/roadmap/issues/133

@Anthchirp that is a completely unrelated and orthogonal feature.

It's actually not unrelated, this roadmap item should actually alleviate some of this pain. Unfortunately, I can't say more at this time.

It's actually not unrelated, this roadmap item should actually alleviate some of this pain. Unfortunately, I can't say more at this time.

@JLLeitschuh, I agree, however the most important question is how the team at Github feels about all of this.
I'm only willing to give a hand because it's clear how beneficial these tools could be to the public... For example, the potential to integrate UIX buttons to generate dependabot configurations directly into the Github UI would be great for newer and casual developers.
However, these are features that need to be something Github is willing to accept. And we haven't heard from them for a while.

I created a GitHub action that mimics what is asked here
It's not as feature-rich as dependabot, but leaving security updates enabled and running this weekly has been sufficient to keep my dependencies updated.

Me too! These Actions also implement this request:

https://github.com/thepwagner/action-update-go
https://github.com/thepwagner/action-update-docker
https://github.com/thepwagner/action-update-dockerurl

There's no detection, but dependencies can be grouped by regular expression - sample.
You can also set a "cooldown" period for particular groups, so the Action can be scheduled daily, without getting ~daily PRs  from projects like https://github.com/aws/aws-sdk-go/tags . I hadn't thought about grouping by the version diff (e.g. patch, minor, major), cool idea!
These actions are unsupported, but maybe a useful starting point.

There is a lot of feedback from the community here but still no word from GitHub or maintainers.
A response would be appreciated, even if you say you are just considering it and may take really long to implement but at least some kind of reply would be nice.

There is a lot of feedback from the community here but still no word from GitHub or maintainers.
A response would be appreciated, even if you say you are just considering it and may take really long to implement but at least some kind of reply would be nice.

Heya, maintainer (and Huber) here.
We'd like to support this, but it would currently indeed take really long to implement so we've not been able to prioritize it. It's on our radar and backlog but I'm not able to give a timeline for when it might land.

I'm not sure how this happened but we got a grouped update from dependabot at Forem https://github.com/forem/forem/pull/11990 :

Bumps flipper-ui, flipper-active_record and flipper. These dependencies needed to be updated together.

@rhymes Its very likely the dependencies that where bumped in your case where strongly connected and dependabot had no option to just bump all of them the same moment.

May I suggest a cumulative approach ?
Dependabot tries to merge dependencies updates in a targeted PR based on a criteria (dependabot dedicated PR, latest opened PR, SEMVER level major/minor/patch as suggested by @dudo, other way?) ; if the update is successfull it keeps stacking. If a specific dependency fails to merge, the PR is rolled back and a new PR is opened for manual investigation.

May I suggest a cumulative approach ?
Dependabot tries to merge dependencies updates in a targeted PR based on a criteria (dependabot dedicated PR, latest opened PR, SEMVER level major/minor/patch as suggested by @dudo, other way?) ; if the update is successfull it keeps stacking. If a specific dependency fails to merge, the PR is rolled back and a new PR is opened for manual investigation.

It would seem that if a configuration option was available to specify which updates are grouped together, it would be able to serve both feature requests.  * should be supported as a wildcard character for convenience and to support grouping "all" as shown below.  (npm package names cannot contain the * character.)  Since dependencies are predetermined, Regex is not necessary and would overly complicate use.
For instance, to group react packages together and storybook packages together:
version: 1
update_configs:
  - package_manager: "javascript"
    directory: "/"
    update_schedule: "daily"
    group_updates:
      - packages:
        - react
        - react-dom
        - @types/react
        - @types/react-dom
      - packages:
        - @storybook/*

Or to group ALL updates in a weekly pull request, you'd just do this:
version: 1
update_configs:
  - package_manager: "javascript"
    directory: "/"
    update_schedule: "weekly"
    group_updates:
      - packages: [*]

We are crying for this!!!

Fine, I have stopped crying and moved to Renovate.

Any update on this? It is particularly important for monorepo dependencies e.g. storybook with dependencies from a single monorepo that generate multiple PRs (which cause merge conflicts on package-lock files).
I'm loathed to move to a competitor but this feature has been coming soon for a couple of years now.

No update on the timeline, it's still something we'd love to support but realistically we won't have time for it in the next couple of months, so it's unfortunately not coming soon.

We would also find this feature very useful.

We're very interested in this feature as well. Details in #4119

Fine, I have stopped crying and moved to Renovate.

Hey there @wangyun1517,
how did Renovate turn out for you?
Would like to get your review on the process :)

We had to disable Dependabot in our monorepo because of this issue. It opened 30+ PRs at the same time, bringing our CI to its knees.
For reference, this was the commit message:
Dependabot floods our CI with pull requests, one for each update.
Dependabot has an open issue for making only one PR:
See dependabot/dependabot-core#1190.

Disable dependabot until this issue is fixed. Note that this way of
fixing it was found in the documentation:
docs.github.com/en/code-security/supply-chain-security/keeping-your-dependencies-updated-automatically/configuration-options-for-dependency-updates#open-pull-requests-limit.

Dependabot may still make PRs for security updates:

This option has no impact on security updates, which have a separate,
internal limit of ten open pull requests.

If that becomes problematic then we can instead generate
an empty dependabot.yml.

Made issue XYZ-123 to re-enable it in the future.

Now that Dependabot is GitHub-native and Dependabot Preview is phased out, I would love to have this feature. Just an example of why:

😅

Yesterday I got my first grouped PR: https://github.com/wemake-services/kira-release/pull/101
Wow! 👍

@sobolevn, wow that looks great! How did you get that? I can't see anything special in your dependabot.yml file?

@sobolevn But that's not a new feature. All these dependencies are linking each other via peer dependencies. For example react and react-dom also usually update in 1 PR if there's a major bump due to peer dependencies.

No, just random.

Really would like this feature too.

Currently I manually create a PR & cherry-pick each commit, resolving conflicts as necessary/. I submit and then let dependabot auto-close so I've caught everything. That bunch will be tested together by our FVTs (in ci/cd). We also do at the beginning of a release cycle (monthly) to allow time for additional flushing out of issues
Any PR that breaks of course needs removing from the batch, along with potentially anything where the info looks dubious.
At least this cuts down on the pr/merge time, but still it is a little disruptive.
I too would find support of batched updates really helpful

In case anyone is interested, I created a quick extension for GitHub CLI that aims to make it slightly easier to deal with mass PRs from Dependabot. It aims to automatically produce a combined PR that groups together as many of the dependency bump PRs as possible, roughly replicating what many of us have been doing by hand.
The extension is here: https://github.com/rnorth/gh-combine-prs, and an example PR can be found here. I won't promise that it's the greatest piece of code (it's fundamentally just a hacked-together bash script 😄), but hopefully it's useful for other people.

@rnorth probably not intenational, but your repo is not accessible (typo or private repo?)   🙃

@WtfJoke OMG, thanks for pointing that out. The repo was indeed private. How embarassing 😅

Similar to rnorth, I also took matters into my own hands - I made a GitHub Workflow that uses git to combine the PRs opened by dependabot - you can trigger it from the Actions tab of your repo. By default it only combines branches where checks have passed - and you can also manually exclude branches by adding the 'nocombine' label to them. You can find it here: https://github.com/hrvey/combine-prs-workflow
Limitation: Because the workflow uses git's automerge, it comes with the same limitations as that - specifically, it tends to fail if the different PRs modify the same line in the .lock to something different - which tends to be the case when something is a shared dependency (e.g. A and B both depend on C, so the two branches bumping A and B both bump C to different versions, so the .lock file can't be auto-merged). So if all your dependencies are closely related (e.g. submodules in a bigger framework), my workflow likely won't help. But if they're mostly unrelated, like mine tend to be, then it can handle it quite well, and if you occasionally run into a problematic merge, you can exclude one of the problematic branches from the combined PR and then merge it separately afterward.

For people who don't want to complicate their pipelines and are ok moving away from dependabot, Renovate bot has such a feature.

@hertzg people are asking for an renovate example YAML file in the comments above.

@WaKeMaTTa Here's my generic Renovate config for grouping packages. It bundles all minor and patches + auto merges them. Major updates are still single PRs with no auto merge. Would love to see this kind of functionality as part of Dependabot because having a self-managed Renovate Bot comes also with some overhead.
{
    "assigneesFromCodeOwners": true,
    "packageRules": [{
            "packagePatterns": ["*"],
            "updateTypes": ["minor", "patch"],
            "groupName": "all non-major dependencies",
            "groupSlug": "all-minor-patch",
            "automerge": true,
            "approvals_required": false,
            "labels": ["dependencies"]
        },
        {
            "packagePatterns": ["*"],
            "updateTypes": ["major"],
            "labels": ["dependencies", "breaking"]
        }
    ]
}

@subesokun You're using outdated properties for some Renovate config options. 😉
Change:

packagePatterns -> matchPackagePatterns
updateTypes -> matchUpdateTypes

Updated Renovate config here:
{
    "assigneesFromCodeOwners": true,
    "packageRules": [{
            "matchPackagePatterns": ["*"],
            "matchUpdateTypes": ["minor", "patch"],
            "groupName": "all non-major dependencies",
            "groupSlug": "all-minor-patch",
            "automerge": true,
            "approvals_required": false,
            "labels": ["dependencies"]
        },
        {
            "matchPackagePatterns": ["*"],
            "matchUpdateTypes": ["major"],
            "labels": ["dependencies", "breaking"]
        }
    ]
}

Thanks for the hint (updated my example too) 👍  Getting the Renovate configs correct is not easy, a lot of trial and error.

PM For dependabot here 👋🏼  If you'd like to monitor any future updates, please watch the issue on the public roadmap. There are no plans for the immediate future since this is a complex problem to solve, but the request remains top of mind.

Unlocking by request so that folks can upvote this as a feature request.
Feel free to leave comments that meaningfully add to the conversation, but avoid comments like "bump" or "we need this" or "is there an update / ETA?" as they waste everyone else's time.
At this time there is no ETA, but I will say we are fully aware this is a high-value feature, and although it could be quarters or years away, I do expect we will ship some sort of grouped update functionality at some point. Subscribe to this issue to get notified whenever that eventually happens.

@jeffwidman definitely agree with not "bumping" the convo but it becomes unavoidable when a stale bot that goes off of activity will close the ticket otherwise.

I don't think if this has been mentioned already, but I think it would be nice to have grouped updates by “topic”, for example I'd love to get grouped updates for dev dependencies, but not for prod ones. Or a group for types dependencies (for TS and Python)

Grouped updates would definitely improve the usefulness of Dependabot, at the moment I’m updating dependencies manually ahead of Dependabots scheduled check to not have many PRs that aren’t mergable on their own, as dependencies rely on each other.
Perhaps an option to specify a set of wildcard namespaces (eg Amazon.*) that cause matching updates to be grouped together.

There's a special case of grouped updates which seems simpler than a more configurable and generalized feature.
Many dependency managers, like bundler for ruby or poetry for python, pair "true requirements data" (Gemfile, pyproject.toml) with a generated lockfile (Gemfile.lock, poetry.lock) which pins the exact package versions. In this case, the update action could just be to run the relevant lockfile regeneration command (bundler update, poetry update).
Dependabot is already somewhat aware of lockfiles (versioning-strategy: lockfile-only). This could even be phrased as an alternative versioning-strategy value, e.g. lockfile-only-grouped.

Is there somewhere less noisy we can follow if we just want to know when some form of grouped updates is implemented?

@davidism To my knowledge, this public roadmap item still represents this feature: https://github.com/github/roadmap/issues/148

Not sure if somebody has already mentioned this, but it would be great to have a method for manually combining two Dependabot PRs (not enabling a "combine-all" option) - e.g. if libraries like react and react-dom have both updated (https://github.com/dependabot/dependabot-core/issues/1190#issuecomment-596163915), or just two internal organization packages, updating from one API to another.

I could be wrong, but it looks like Dependabot is now exploring the ability to have "Grouped" dependency updates.
https://github.com/microsoft/reactxp/pull/1389, https://github.com/microsoft/reactxp/pull/1384 and others!

Noticed the same thing. But maybe for security alerts only? Glad to see it happen :D

It seems like it is only for group updating ancestor dependencies yet. But it is a good start nevertheless!!

What might the config file look like to opt into this? It doesn't have to be complicated:
version: 2
updates:
  - package-ecosystem: 'npm'
    directory: '/'
    schedule:
      interval: 'weekly'
    batching: grouped # individual / grouped / security-batched / etc. ...

I'd love to see this as well, this is a blocker for us

What might the config file look like to opt into this? It doesn't have to be complicated:
version: 2
updates:
  - package-ecosystem: 'npm'
    directory: '/'
    schedule:
      interval: 'weekly'
    batching: grouped # individual / grouped / security-batched / etc. ...

I like that.
Either way, I hope something gets implemented soon.

This is an example of why this feature is important:

In .NET world, this will never work given they all need to be updated simultaneously.

Yeah this needs to be much more configurable.  E.g. doesn't make sense to test updating MongoDB.Driver on 1/10 csproj without updating any other MongoDB.* pacakges within the same PR - it will simply fail to build.

Same reason for me as well.

The lack of grouped updates means that we end up using a huge number of Actions minutes every time we merge a Dependabot PR, and the resulting queue of jobs means that our repo is essentially frozen for hours as we wait for the queue to clear. I ended up disabling rebasing in my repo, which I hope will help with this problem.

A gentle reminder of what I said previously:

Unlocking by request so that folks can upvote this as a feature request.
Feel free to leave comments that meaningfully add to the conversation, but avoid comments like "bump" or "we need this" or "is there an update / ETA?" as they waste everyone else's time.
At this time there is no ETA, but I will say we are fully aware this is a high-value feature, and although it could be quarters or years away, I do expect we will ship some sort of grouped update functionality at some point. Subscribe to this issue to get notified whenever that eventually happens.

The last eight comments on this ticket are effectively spam for everyone else who's subscribed to this ticket for updates... if this continues, I'll have to lock the issue again.

Unlocking by request so that folks can upvote this as a feature request.
Feel free to leave comments that meaningfully add to the conversation, but avoid comments like "bump" or "we need this" or "is there an update / ETA?" as they waste everyone else's time.
At this time there is no ETA, but I will say we are fully aware this is a high-value feature, and although it could be quarters or years away, I do expect we will ship some sort of grouped update functionality at some point. Subscribe to this issue to get notified whenever that eventually happens.

@jeffwidman Let me know if I can help with testing if the feature is in works

I think the strategy is not straight-forward.  Grouping everything in one PR would help, since the PR can be edited to revert some of the updates to get a partial set.
However configuration or UI to expiate this would really help e.g.
updateGroups:
   group1:
     - MongoDb*
     - System.Bson*
     - Stackexchange.Redis*
   group2:
     - Microsoft.CodeAnalysis*
   group3:
     - *

so that there are 3 potential PRs that could be opened at any one time.

However configuration or UI to expiate this would really help e.g.
updateGroups:
   group1:
     - MongoDb*
     - System.Bson*
     - Stackexchange.Redis*
   group2:
     - Microsoft.CodeAnalysis*
   group3:
     - *

@dave-yotta with YAML you can have nested/multidimensional arrays without having to name them, which would make maintaining such a configuration easier. For example:
updateGroups:
  -
    - MongoDb*
    - System.Bson*
    - Stackexchange.Redis*
  -
    - Microsoft.CodeAnalysis*
  -
    - *

I'm just giving some gist...but I think named groups would be nice to see e.g. on the PR title.  Maybe the model gets deeper as there are other things possibly relevant such groups forward-looking:
updateGroups:
 - name: Databases
   frequency: .... other stuff
   patterns:
    - MongoDb*
.....

etc...

@brcarp But also quite unreadable IMO. I think named groups would be better.

Hello 'Miroslav,
Thank you for contacting us!  The Notable office is currently closed in observance of the Martin Luther King Jr. Holiday.
We will resume normal business hours on Tuesday, January 17th from 9am to 9pm EST and look forward to assisting you at that time.
Happy Holidays!
Best Regards,
The Notable Team

I forgot you could reply by email 💀

Hi, we are currently in a similar situation as bajtos above. Due to company reasons, we are moving from renovatebot to dependabot and we really miss the ability to group pull requests by scope in the same repository.
For example, instead of having 5 pull requests that update patch versions of 5 different dependencies, causing builds for every PR and rebuilds after every merge, renovatebot was able to group them all into a single PR. In this case, we except patch versions to not break anything so we are much more lenient on testing them.
This would make dependabot much easier to use and wouldn't strain our pipeline as much.

At least a simple option to group all minor and patch updates into the same PR would be very helpful already.

I'm going to lock this for now to spare the inboxes of folks who are subscribed primarily for updates.

👋🏻 Hey everyone, I'm the Product Manager for Dependabot Updates. If you would be interested in providing your feedback on Dependabot and answer a bunch of questions about how you use it today, please feel free to book a time on my calendar: https://calendly.com/carogalvin/30min
In particular, we are currently researching how Security Updates should behave for grouping, and recruiting for the private beta of grouped Version Updates in May. Please feel free to book a slot or email me at carogalvin[at]github.com if you're interested!

GREAT NEWS! In case you haven't seen it already, grouping rules for version updates is now in PUBLIC BETA which means you will be able to set up grouping rules for any repo now!
Since we are in public beta, you may notice some instability or changes in behaviour without notice. If you encounter any bugs, please file new issues for them and the team will take a look.
You can see the docs on how to set these up here: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups
If you have any feedback, please feel free to email me at<EMAIL_ADDRESS>or set up a meeting with me by picking a convenient time in my calendar here.

We're working on adding support for grouping on SemVer levels, some details here which links to a form you can fill out for private beta access for the feature

FYI for anybody using groups, we discovered this bug that may result in you seeing multiple grouped and ungrouped PRs for the same updates. We're working on a fix now, and you can track the linked issue for updates.

Good news! Grouped updates for Dependabot version updates is officially generally available 🥳
Read our blog post here: https://github.blog/2023-08-24-a-faster-way-to-manage-version-updates-with-dependabot/
See the docs here: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file#groups
With that, we will close this issue. Thanks everyone for your feedback!


Speed up UCAS spec

Context
This is one of the slower specs in the test suite. Running the RSpecDissect profiler against it indicates that this is largely because of the amount of time it spends in let (ie - creating records in the database).
The approach recommended by the test-prof docs for dealing with this is using the let_it_be/before_all methods provided by the gem. These methods hook into RSpec's before(:context) to allow database records to be set up once before executing examples in the group (rather than recreating them for each example). Any modifications to these records are automatically rolled back between examples, and an option is available to automatically reload the ActiveRecord object as well.
The unfamiliar syntax and new behaviour can potentially make the spec more difficult to follow. Raising this PR for discussion to weigh up the performance improvement vs impact on clarity.

Changes proposed in this pull request

Use the let_it_be/before_all methods in ucas_matched_application_spec. This change reduces the runtime from 6.72 seconds to 0.5 seconds (tested on a machine with an AMD Ryzen 7 3700X).
Guidance to review

Is it still reasonably clear what's going on in the spec?
We can specify an alias for let_it_be if we think that would help clarify things.

Link to Trello card
https://trello.com/c/ySrLogAF

Things to check

[x] This code does not rely on migrations in the same Pull Request
[x] If this code includes a migration adding or changing columns, it also backfills existing records for consistency
[x] API release notes have been updated if necessary
[x] New environment variables have been added to the Azure config

Closing in favour of https://github.com/DFE-Digital/apply-for-teacher-training/pull/3871. That gets us most of the way in terms of performance without needing to introduce new syntax. Can revisit this technique if there are specs that need heavy, one-off DB setup but are difficult to stub without affecting clarity.

Closing in favour of https://github.com/DFE-Digital/apply-for-teacher-training/pull/3871. That gets us most of the way in terms of performance without needing to introduce new syntax. Can revisit this technique if there are specs that need heavy, one-off DB setup but are difficult to stub without affecting clarity.


Alignments extending past the end of reference sequences

Hi,
I have seen cases where alignments extended past the end of reference sequences (i.e. start from the stated position, following the CIGAR operations and positions past the end of the reference sequence are used). These are invalid and should not be produced.
Best regards,
German

Hi German,
Thanks for reminding, I will debug rHAT as soon as possible.
Best,
Dengfeng Guan


hdf5 1.14

not waiting for https://github.com/conda-forge/fenics-feedstock/pull/174

needs to wait for https://github.com/conda-forge/hdf5-feedstock/pull/202

@conda-forge-admin please rerender


Predicate testing forces dependency on Minidev json-smart

When using a Predicate expression to filter a value, a new Configuration is built with no mappingProvider (ValueNode line 808). This forces the ConfigurationBuilder to call getEffectiveDefaults(). If an alternate mappingProvider was set on the context, but not set as a default, this will be ignored and the default minidev JsonReader will be used; if it isn't on the classpath this causes a java.lang.NoClassDefFoundError: net/minidev/json/writer/JsonReaderI exception.
Here's a simple test with a predicate expression:
@Test
public void testJsonPathWPredicate() throws Exception {
    final Configuration configuration = Configuration.builder()
            .jsonProvider(new JacksonJsonProvider())
            .mappingProvider(new JacksonMappingProvider())
            .build();

    final String json = "{\"outerKey1\": {\"innerKey1\": \"value\", \"innerKey2\": \"foo\"}}";
    final String predicateExpr =<EMAIL_ADDRESS>== 'foo')].innerKey1";
    final List<String> result = JsonPath.using(configuration).parse(json).read(predicateExpr);
    assertEquals(Lists.newArrayList("value"), result);
}

If you do not have minidev json-smart on your classpath, this test will throw
java.lang.NoClassDefFoundError: net/minidev/json/writer/JsonReaderI

    at com.jayway.jsonpath.internal.DefaultsImpl.<init>(DefaultsImpl.java:17)
    at com.jayway.jsonpath.internal.DefaultsImpl.<clinit>(DefaultsImpl.java:15)
    at com.jayway.jsonpath.Configuration.getEffectiveDefaults(Configuration.java:48)
    at com.jayway.jsonpath.Configuration.access$000(Configuration.java:34)
    at com.jayway.jsonpath.Configuration$ConfigurationBuilder.build(Configuration.java:229)
    at com.jayway.jsonpath.internal.filter.ValueNode$PathNode.evaluate(ValueNode.java:808)
    at com.jayway.jsonpath.internal.filter.RelationalExpressionNode.apply(RelationalExpressionNode.java:37)
    at com.jayway.jsonpath.internal.filter.FilterCompiler$CompiledFilter.apply(FilterCompiler.java:400)
    at com.jayway.jsonpath.internal.path.PredicatePathToken.accept(PredicatePathToken.java:76)
    at com.jayway.jsonpath.internal.path.PredicatePathToken.evaluate(PredicatePathToken.java:46)
    at com.jayway.jsonpath.internal.path.PathToken.handleObjectProperty(PathToken.java:81)
    at com.jayway.jsonpath.internal.path.PropertyPathToken.evaluate(PropertyPathToken.java:79)
    at com.jayway.jsonpath.internal.path.RootPathToken.evaluate(RootPathToken.java:62)
    at com.jayway.jsonpath.internal.path.CompiledPath.evaluate(CompiledPath.java:53)
    at com.jayway.jsonpath.internal.path.CompiledPath.evaluate(CompiledPath.java:61)
    at com.jayway.jsonpath.JsonPath.read(JsonPath.java:187)
    at com.jayway.jsonpath.internal.JsonContext.read(JsonContext.java:164)
    at com.jayway.jsonpath.internal.JsonContext.read(JsonContext.java:151)

I believe this could be fixed by changing ValueNode line 808 to explicitly set the mappingProvider (even if it is never used).
Configuration c = Configuration.builder().jsonProvider(ctx.configuration().jsonProvider()).options(Option.REQUIRE_PROPERTIES).build();

to
Configuration c = Configuration.builder().jsonProvider(ctx.configuration().jsonProvider()).mappingProvider(ctx.configuration().mappingProvider()).options(Option.REQUIRE_PROPERTIES).build();

I also hit this issue when running in OSGi environment. I even considered installing json smart library to work around it, but finally I ended up dropping the predicates and manually checking the elements. I would love to see this fixed.

I'd say this is a non-issue. IMO, json-smart is a dependency and if you exclude it from your classpath, things stop working.

If we had a way to parse JSON within the query in a provider-independent fashion, we could make the dependency optional again. Of course, we could just copy over the relevant code from the json-smart library, but I wouldn't consider that an optimal solution.
I'm beginning to wonder how much work it would be to port the query parsing to ANTLR...

I don't understand why the fix I proposed in the initial ticket is not appropriate.
I have put a non-default mappingProvider on the Configuration. It is being used everywhere except for predicate checking, where it isn't being used and the default is used instead. But it easily could be used. The fix is simple. Is there some reason that should not be done?

See #275. Some providers do not support parsing JSON with single quotes. But since the JsonPath specs allow that, we cannot use the provider.


Use default tabbing identifiers for windows

This fixes a minor bug I found with tabbing behavior: if you create two separate windows (⌘N ⌘N), Xi won't let you drag tabs between them. Apparently this is because the windows are given different tabbing identifiers—if I remove that code, the problem goes away. As far as I can tell, the new tab and new window behavior remains exactly the same.

Awesome, I would be very happy to delete this hack. IIRC the original problem was with correctly associating new tabs with the window that had received the event, but it definitely didn't feel quite right.

Oh, I remembered something else about this: behaviour is highly dependent on the user's setting for 'Prefer tabs when opening documents'. Did you test this with that on always/manual?

Played around with this a little bit.
The first thing I noticed: after opening two new windows (after closing all windows), and with the second window focused, cmdT creates a new tab in the first window, not in the active window.

@ryanbloom yea, we're on the same page. The mess you're looking at is the best I could come up with when we first added tab support, which bought us n. 2 but cost us n. 4. My design at the time was to have cmd+N open a new tab if that was the user's system preference, but I can go either way on that?

Haven't checked the behaviour yet but agree with having the option to always open a new window. Sometimes that just is what I want, even if I normally prefer otherwise. One thing worth considering is the exact shortcut for that: TextEdit uses ⌥⌘N whereas Atom uses ⇧⌘N. Xcode on the other hand is a bit confusing since it doesn't let you create scratch buffers to begin with but always requires you to go through the file creation wizard. It uses ⌥⌘N to create a new group and ⇧⌘N for a new project so if we want to follow the Atom/Sublime model of one window per project/workspace then ⇧⌘N is definitely the way to go. On the other hand there's TextEdit which doesn't have the concept of 'projects' (nor a folder sidebar).

OK, playing around with this I made some observations:

⌥⌘N seems to be accepted for 'new window' despite not being present in the menu (this is the case in TextEdit as well - but not in Atom, where it is also accepted as a new document shortcut but opens a tab instead of a window/workspace - so I suppose it has something to do with Cocoa itself)
Dragging tabs into other windows as tabs still only works if tone is 1) dragging a tab, not a window 2) the receiving window already has a tab bar.

Regarding latter point, 2) can probably be fixed by forcing a tab bar on all windows when one of our windows is being dragged. 1) is more difficult since we'd have to turn the dragged window into a tab automagically. There's also the case of dragging a window that already has tabs - how should it behave when dragged as a tab into another window? Should it be shorthand for moving all tabs to that window? Should it be allowed at all (i.e. not trigger the tab bar on other windows)?
As for my previous comment, the exact behaviour there is mostly a matter of what kind of editor xi aims to be. Given the heavy focus on code I guess the folder/workspace -centric approach makes more sense.
As for the changes in here, ⌘O now honours the system setting and ⌘N and ⌘T do what they say on the tin so LGTM, unless you want to fix the tab/window dragging issue here as well.

(also just noticed I've duplicated some of @jansol's comments; great minds etc etc)