Metadata categorical types are bit too strict

Addition Description
Right now if you have a column of unique ids (i.e. host_subject_id) that are integer valued but if you want to pass it in as a categorical type, qiime2 will throw an error that looks like below
Current Behavior
 (2/2) Invalid value for '--m-replicates-file': Metadata column is of type
  'numeric', but expected Categorical.

Proposed Behavior
A better way to handle this is if the metadata column is expected to be categorical, then it should be forced to be categorical by converting all of the values in the column to strings.
Comments
This problem isn't unique to the q2-batch that I'm putting together, but really all of the differential abundance tools that use the metadata column paradigm. We're planning on automating these types of diff abundance analysis on dozens of studies that use the qiita standards for metadata; where these sorts of errors induce additional user / developer burden.  Having more relaxed handling of categorical types would significantly ease user burden moving forward.
EDIT:
The current solution to this problem is to directly modify the categorical columns.  My current solution is to append 'X' in front of the variables via
import argparse
import pandas as pd

parser = argparse.ArgumentParser()
parser.add_argument(
    '--metadata-file', help='Metadata file', required=True)
parser.add_argument(
    '--categorical-column', help='Categorical column', required=True)
parser.add_argument(
    '--output-file', help='Output metadata file', required=True)

args = parser.parse_args()

md = pd.read_table(args.metadata_file, index_col=0)
md[args.categorical_column] = md[args.categorical_column].apply(lambda x: 'X' + str(x))
md.to_csv(args.output_file, sep='\t')

Of course, it would be more elegant to have the type casting handled internally in qiime2 metadata.  If we agree that this is useful, I'll be happy to push in a pull request.

Ha! We are actually discussing this same "feature" in Qiita, for reference: https://github.com/qiita-spots/qiita/issues/2947. To be fair, I completely forgot about the numeric host_subject_ids or sample_name, which happens ofter.
Something I considered to fix in Qiita was to add a GUI for the sample information file where you can change the #q2:types of the columns (perhaps by default categorical). Extra from development it has other internal management issues (saving backups, etc).
Anyway, thought it might be beneficial to link both issues.

What about two options that are added to any plugin with a Metadata parameter. Namely --p-as-categorical and --p-as-numeric this way users can have a say on how metadata is interpreted depending on the plugin.  I know something like this would get complicated for things like the Python interface. But what I like about this approach is that type-inferences would be visible in the provenance information.

Clearly this is a topic of interest to a lot of us. 👍

I like @ElDeveloper idea. Now, how can we expand it to commands where you don't list the columns of interest in the call, like alpha group significance ...

But what I like about this approach is that type-inferences would be visible in the provenance information.

I think that's exactly what the typed-columns I shared above dose - provenance captures the metadata provided, including the type annotations provided by the user.

Clearly this is a topic of interest to a lot of us.

For sure! Like I said, I am interested in @mortonjt's initial suggestion! I wanted to share the current column typing spec, because it looked like @mortonjt wasn't aware that users can directly control how that inference is performed, right now.

I think that's exactly what the typed-columns I shared above does - provenance captures the metadata provided, including the type annotations provided by the user.

🤯 wait what? That's cool, I didn't know about this feature. Where is that info displayed/registered though? For example in the provenance for this artifact I only see the variable names not the types.

Unfortunately it isn't rendered anywhere in q2view (yet!), but you can grab it out of the archive:
wget https://docs.qiime2.org/2021.2/data/tutorials/pd-mice/metadata.qzv
qiime tools extract --input-path metadata.qzv --output-path metadata
head metadata/4dbb2457-180f-40f0-9a8d-e37538c48b3b/provenance/action/input.tsv

This metadata is tracked ancestrally, too, so you'll have access to it (via the provenance dir) in downstream outputs, too.

I was aware of the typed columns; I think that was a great addition!
One of the reasons why I raised this was because adding typed columns was looking tedious (we've got many metadata files that we're juggling).
Since this appears to be a recurring use case, I wonder if we could make this process even more straightforward by forcing the types. If the input type MetadataColumn[Categorical] is already specified, then it could be useful if this typing could be automatically enforced on the metadata.

One of the reasons why I raised this was because adding typed columns was looking tedious (we've got many metadata files that we're juggling).

Personally I would go for the option that doesn't mutate the input data!

I wonder if we could make this process even more straightforward by forcing the types

I think there is certainly merit for making this work this way in the case of MetadataColumn[Categorical] - any value type can be represented as categorical data (I think). I'm less certain about the MetadataColumn[Numerical] case, or even future as-yet-written column types, though.
Question - do you think a utility for this casting would get you what you need? For example, we could expose parameters in q2-metadata's tabulate viz that would accept an optional list of "columns to make categorical" or "columns to make numerical" - then that metadata could be yanked out of the viz/provenance and used downstream. Just spitballing ideas here. Or we could make this util a built-in, so an interface like q2cli could have a qiime tools cast-metadata command, or something like it.

This metadata is tracked ancestrally, too, so you'll have access to it (via the provenance dir) in downstream outputs, too.

So cool! This seems like an opportunity for expanding on q2view's capabilities.

I think there is certainly merit for making this work this way in the case of MetadataColumn[Categorical] - any value type can be represented as categorical data (I think). I'm less certain about the MetadataColumn[Numerical] case, or even future as-yet-written column types, though.

Come to think about it this would make more sense than adding new options. Some error handling would need to be included to raise errors when the data can't be cast to numeric, or when all the categorical values are unique, etc. 👍

Errors would be great for the MetadataColumn[Numerical] use case; we've had users that run into non-intuitive errors when running Songbird on continuously valued data.  The worst is when there is a silent error, and the program will process numerical data, but treat it as categorical.

we've had users that run into non-intuitive errors when running Songbird on continuously valued data

Can you point us at some issues or discussions somewhere? Is this related to QIIME 2, or to songbird? I cursory glance in the songbird code looks like you don't really use QIIME 2 metadata in any of the Actions defined there, instead it looks like Metadata is converted to pd.Dataframe early and often - at that point I'm not too sure what we can do for yah - but there are some APIs on metadata that might make your life a little bit easier - we should chat more about improvements that could be made to Songbird using the existing API!

The worst is when there is a silent error, and the program will process numerical data, but treat it as categorical.

Interesting - this is the first report of a silent metadata error I have heard - that must be a frustrating situation. Please share some more specifics so that we can help you get to the bottom of it.

Not a problem.  Below is an example of a silent user error with continuous variables (I've received this question a number of times over email as well).
https://github.com/biocore/songbird/issues/151
I agree, it would be great if we could use the Metadata API; but I was under the impression that the Metadata API does not offer support for Patsy formula formatting (I see that there is a bit of overhead code in q2-longitudinal to handle this). Would love to follow up discussions on this; perhaps its worthwhile to raise a separate issue for patsy support?

Hi @thermokarst I've opened up a pull request.
Yes, your comment regarding the header would probably help - but in all honestly, there are too many competing standards for metadata file types.  That change would break all of my code that relies on qiita metadata files for instance.
I've opened up a WIP pull request at https://github.com/qiime2/qiime2/pull/588
I think this minor change will be advantageous for q2, since this is a common convention in the ML community anyways.

Hi @thermokarst , thank you for following up.  The songbird issue is a separate one regarding numerical types rather than categorical. The PR I've opened allows for integer-valued columns to be used as categorical variables.   I'm currently running into this issue with q2-sample-classifier as well - should I raise an issue there? CC @nbokulich
Here's another question, if one type under the q2:types directive is specified, do they all need to be specified?
One of the reasons why I haven't jumped on this train yet is because many of the metadata files that I'm dealing with have hundreds of columns - manually specifying types is quite tedious.

I'm not sure about the q2-sample-classifier issue, I'll wait to hear what @nbokulich says.
RE the q2:types directive, here is another passage from the doc linked to above (emphasis mine):

The row’s first cell must be #q2:types to indicate the row is a comment directive. Subsequent cells may contain the values categorical or numeric (both case-insensitive). The empty cell is also supported if you do not wish to assign a type to a column (the type will be inferred in that case). Thus, it is easy to include this comment directive without having to declare types for every column in your metadata.

Hey @mortonjt thanks for pulling me in. RE: q2-sample-classifier tbh I think I had this same discussion with @thermokarst a few years ago when developing that plugin (using the types directive was the solution at the time). Indeed this is a common issue, since many times class labels are numerically encoded.
We could consider moving this discussion over to q2-sample-classifier, but it sounds like this is not isolated to that plugin so a solution there would not help us unless if that is your primary target for this.
My two cents on #588 : relaxing MetadataColumn[Categorical] to accept int seems reasonable, but maybe it would be worth building in additional validation to check that int data look categorical, e.g., are not all unique values (or mostly unique values). Would that just make metadata validation too inefficient?

e.g., are not all unique values (or mostly unique values). Would that just make metadata validation too inefficient?

@nbokulich, good idea ... my two cents, I guess the speed will depend on how it is done. A "quick" way is that the number of groups should be at most X% of the samples (taken from here: https://github.com/qiita-spots/qiita/issues/1595); for example, if we select 10% basically a categorical variable with 5 values should at least have 50 samples in the full dataset (independent of the distribution of those), 4 values 40 samples ...

I think making sure that 10% of metadata values aren't unique is a reasonable check - I can incorporate that in the PR. RE number of groups, that may require additional discussion since it isn't clear what threshold would be appropriate, since that could depend on the method right?  For instance, K-nearest neighbors won't care if about the number of class instances.

I'm not a big fan of the 10% approach - I think the current contract of "all or nothing" makes for a very predictable API. I think if we wanted to pursue the 10% approach, we should mirror the current validation level approach in the Format API, that way we don't have a situation where some column types are treated differently from others (that sounds like a recipe for pain, to me). For those not aware, the Format API allows for formats to define two levels of validation, and different portions of the framework etc can take advantage of that when they need to (import performs full validation, while saving Results is minimum, if I recall correctly). For record-based formats (like Metadata) this usually looks like:

min: validate the first 10-100 rows/records
max: validate all of the rows/records

Alternatively, should we consider expanding the typing directive vocabulary here? The typing directive supports "numeric" and "categorical" data - is this an indication that a new type is needed? It just seems to me like these complaints are all solved relatively easily with the existing API, but I could certainly be missing something. Thanks for humoring me!

yeah I also don't like the 10% approach — it makes too many assumptions about the distribution. Even with a subsample I still worry that this will make metadata validation too inefficient.
It also sounds like the use case here is centered on q2-sample-classifier and related plugins, but we need to keep in mind that we must serve general use cases since Metadata touches everything... e.g., this comment:

For instance, K-nearest neighbors won't care if about the number of class instances.

makes me think we could/should address at the plugin level (since loosening type validation could have unintended consequences for other plugins, e.g., q2-diversity), or just the user level (which is what I do when running similar loops over a large number of int columns with q2-sample-classifier).

From what I'm seeing from this discussion there are two possible routes
Option 1 : Loosen the metadata types, but with careful validation.
Option 2 : Have everything go through the q2 directives as @thermokarst suggests.
The main reason why I haven't gone through Option 2 myself is because I'm still unfamiliar with the interface.
Most of my metadata curation is done through jupyter notebooks, and I haven't spent the time to figure out how to programmatically format the q2 directives with writing the metadata files to disk.  I bet this is trivial, but I haven't seen any documented APIs on how to do this.  If there is write_metadata(dataframe : pd.DataFrame, types : dict) function in say q2-metadata, I think this would resolve my current issue.  Thoughts @thermokarst ?

I really like the idea of expanding some of the built-in tools - give me a few days to put my thoughts together on that, thanks @mortonjt!

Hey there @mortonjt, I put together a brief example which might help demonstrate how you can accomplish your proposed write_metadata, today, by casting your dataframe values prior to constructing your QIIME 2 Metadata:
import os
import qiime2
import pandas as pd
import tempfile

def roundtrip_md(md):
    with tempfile.TemporaryDirectory() as fh:
        fp = os.path.join(fh, 'md.tsv')
        md.save(fp)

        with open(fp) as written:
            print('\ncontents of md file:')
            print(written.read())

df = pd.DataFrame({'foo': [1, 2, 3], 'bar': [4, 5, 6]},
                  index=pd.Index(['a', 'b', 'c'], name='id'))
print(df)
print(df.dtypes)

md1 = qiime2.Metadata(df)
print(md1)
roundtrip_md(md1)

md2 = qiime2.Metadata(df.astype({'foo': 'str'}))
print(md2)
roundtrip_md(md2)

output
    foo  bar
id
a     1    4
b     2    5
c     3    6
foo    int64
bar    int64
dtype: object
Metadata
--------
3 IDs x 2 columns
foo: ColumnProperties(type='numeric')
bar: ColumnProperties(type='numeric')

Call to_dataframe() for a tabular representation.

contents of md file:
id	foo	bar
#q2:types	numeric	numeric
a	1	4
b	2	5
c	3	6

Metadata
--------
3 IDs x 2 columns
foo: ColumnProperties(type='categorical')
bar: ColumnProperties(type='numeric')

Call to_dataframe() for a tabular representation.

contents of md file:
id	foo	bar
#q2:types	categorical	numeric
a	1	4
b	2	5
c	3	6

In this case, any files that are written using Metadata.save() will already have the correct q2:types directive, already.

Awesome, thanks @thermokarst ! Since it looks like the existing API already accommodates this proposed change, I'm going to close this issue.

@thermokarst, thank you for the code.
If I understand correctly, the idea of your code is that it should be added in each individual plugin, correct? If yes, what do you think about adding to all q2 a flag like what has been suggested before so they can be exposed automatically: https://github.com/qiime2/qiime2/issues/568#issuecomment-803000730? If not possible, is there a "quick" way to find all the plugins that require a specific column as input so we can add that flag?
If you are wondering why I still think this is important in adding to all available plugins (and I'll be willing issuing PRs for this) is that  IMOO there is one important point missing based on my own analyses and other Qiita/QIIME 2 users: it would be ideal if it could be possible to control how to use a given metadata column (categorical or numeric) without having to actively change the #q2:types row AKA change the behavior via parameters in the command. For example, a numeric column representing time, could be used as numeric in a PCoA plot but also as different groups in distance matrix group significance.

Jumping in late here, but I think @thermokarst's solution is geared more towards something a user (or interface) would perform, instead of the plugin. More or less, it seems that metadata's typing feature handles this situation well enough, there's just maybe not enough documentation in the user tutorials on using it in practice.
If there were some kind of on the fly type-casting for columns, I think it would make sense as an extended syntax in a CLI, rather than something added as a parameter (no need to search and modify plugins here). The interface is after all, ultimately responsible for providing the metadata to the plugin, and there's no reason it couldn't do type coercion on request. (If you think about it, that's exactly what Matt's example above is doing.)
Re: your point, I think time is a good example of something where we would want something like a Date or Time type. Someone would have to come up with rules for when a string is a member of that type, but we could consider such a type to be both numeric and categorical pretty easily, although I expect binning time would be the more typical way to convert it to categorical (technically ordinal/still time).
Similarly, more granular types are possible (i.e. "levels of measurement"), but there isn't much consensus on their utility, and as most larger systems (like R and Galaxy) get by with just the two (recently time is common), so we haven't exactly jumped to realize those more complex hierarchies.

+1 for Date/Time type, since it comes with concrete units.
On Fri, Jun 11, 2021 at 10:40 AM Evan Bolyen @.***>
wrote:

Jumping in late here, but I think @thermokarst
https://github.com/thermokarst's solution is geared more towards
something a user (or interface) would perform, instead of the plugin. More
or less, it seems that metadata's typing feature handles this situation
well enough, there's just maybe not enough documentation in the user
tutorials on using it in practice.
If there were some kind of on the fly type-casting for columns, I think it
would make sense as an extended syntax in a CLI, rather than something
added as a parameter (no need to search and modify plugins here). The
interface is after all, ultimately responsible for providing the metadata
to the plugin, and there's no reason it couldn't do type coercion on
request. (If you think about it, that's exactly what Matt's example above
is doing.)
Re: your point, I think time is a good example of something where we would
want something like a Date or Time type. Someone would have to come up
with rules for when a string is a member of that type, but we could
consider such a type to be both numeric and categorical pretty easily,
although I expect binning time would be the more typical way to convert it
to categorical (technically ordinal/still time).
Similarly, more granular types are possible (i.e. "levels of
measurement"), but there isn't much consensus on their utility, and as most
larger systems (like R and Galaxy) get by with just the two (recently time
is common), so we haven't exactly jumped to realize those more complex
hierarchies.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
https://github.com/qiime2/qiime2/issues/568#issuecomment-859706941, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AA75VXNT2GYM2SNYDPLRVU3TSI4BHANCNFSM4ZPGCMQA
.

Thank you @ebolyen. I think adding specific types, would help but not completely fix the issue: users will still need to cast for specific scenarios or data by changing the #q2:types row. Additionally, we could end up with more data types that we really want to deal with. Another example that comes to mind: serialized "numeric" host subject ids - this could represent the categorical host unique id but also, as they are serialized, a proxy to when the sample was created in the system which could be used as a numeric value and potentially useful for other analyzes.

I think it would make sense as an extended syntax in a CLI
I think I agree with this, one option is that whenever MetadataColumn[Numeric |Categorical] is used, there is an automatic flag added, like --force-case (or whatever you think best), so the casting is forced; then this will be available for users and stored in the provenance. The user has the flexibility to do it and it is keep in the records. What do you think?

@ebolyen, just wondering if you have any thoughts about the proposed flag; if you + other devs agree, I can work on a PR to add it.

Hey @antgonza,
We're actually about to start a project to make this a little simpler on the command line. It isn't exactly a flag that is added everywhere, but it will make this simpler for users hopefully.

Thank you so much! Please let me know if you need help with code, reviews or testing ... looking forward to this addition.


Unexpected character: '?' @ line 1, column 1.      ?      ^

Current Behavior
when i upgrade my flutter i got some issues in old application it does not show this error only in some
Expected Behavior
Launching lib\main.dart on CPH2239 in debug mode...
Running Gradle task 'assembleDebug'...
FAILURE: Build failed with an exception.

Where:
Settings file 'E:\Completed\Hiredge\jobkar_pro\android\settings.gradle' line: 1

What went wrong:
Could not compile settings file 'E:\Completed\Hiredge\jobkar_pro\android\settings.gradle'.

startup failed:
settings file 'E:\Completed\Hiredge\jobkar_pro\android\settings.gradle': 1: Unexpected character: '?' @ line 1, column 1.
?
^

1 error

Try:

Run with --stacktrace option to get the stack trace.
Run with --info or --debug option to get more log output.
Run with --scan to get full insights.
Get more help at https://help.gradle.org.

BUILD FAILED in 794ms
Error: Gradle task assembleDebug failed with exit code 1
Context (optional)
pluginManagement {
    def flutterSdkPath = {
        def properties = new Properties()
        file("local.properties").withInputStream { properties.load(it) }
        def flutterSdkPath = properties.getProperty("flutter.sdk")
        assert flutterSdkPath != null, "flutter.sdk not set in local.properties"
        return flutterSdkPath
    }
    settings.ext.flutterSdkPath = flutterSdkPath()

    includeBuild("${settings.ext.flutterSdkPath}/packages/flutter_tools/gradle")

    repositories {
        google()
        mavenCentral()
        gradlePluginPortal()_picker
    }
}

plugins {
    id "dev.flutter.flutter-plugin-loader" version "1.0.0"
    id "com.android.application" version "7.3.0" apply false
    id "org.jetbrains.kotlin.android" version "1.9.23" apply false
    id "com.google.gms.google-services" version "4.4.1" apply false
}

include ":app"

//include ':app'
//
//def localPropertiesFile = new File(rootProject.projectDir, "local.properties")
//def properties = new Properties()
//
//assert localPropertiesFile.exists()
//localPropertiesFile.withReader("UTF-8") { reader -> properties.load(reader) }
//
//def flutterSdkPath = properties.getProperty("flutter.sdk")
//assert flutterSdkPath != null, "flutter.sdk not set in local.properties"
//apply from: "$flutterSdkPath/packages/flutter_tools/gradle/app_plugin_loader.gradle"

Steps to Reproduce
try so many times did work
Gradle version
8.5
Build scan URL (optional)
No response
Your Environment (optional)
No response

Your issue appears to be a question about how to use Gradle. They should be asked in a different channel, so we will close it. Your question might be answered in Gradle documentation or on our community Slack or forums. Find links to these resources on help.gradle.org.
If you think our analysis is wrong, please provide us with more detailed information explaining why.

Please check your settings.gradle as advised in the message.


[DON'T MERGE]Issue #268 - Support creating secret configs in UI

Please don't merge since required API changes are not merged yet.

My API changes are now in master and 1.0


Add version to private key

Codecov Report

:exclamation: No coverage uploaded for pull request base (update-to-testnet23@10c95ab). Click here to learn what that means.
Patch has no changes to coverable lines.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
Additional details and impacted files
@@                  Coverage Diff                   @@
##             update-to-testnet23     #527   +/-   ##
======================================================
  Coverage                       ?   40.05%
======================================================
  Files                          ?       15
  Lines                          ?     1126
  Branches                       ?        0
======================================================
  Hits                           ?      451
  Misses                         ?      629
  Partials                       ?       46

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


Use .env file for environment/settings variables

Stemming from discussion in #21, I'd like to consider using a single .env file to control environment variables and a number of Django settings (e.g. TIME_ZONE) options.

python-dotenv looks like a good candidate to help with support for this.
Need to consider and thoroughly test all deployment methods.
It would be nice to cut down on the number of template settings files for various deployments, but some of them use their own special environment variables.

After toying with this I decided not to go full .env file dependence as it causes lots of friction with some of the deployment methods that don't care about untracked files. dd4455178a5fb34324756811981a7fb72c7db79b establishes some environment variable controlled settings and documentation. And I still want to add an optional .env file check.

3c5aba9ae77ca9127efb981e0ec1b15f568e56f9 adds an .env file check and relevant documentation. 🎉


enable mode to assert out on templated vars in the PDOM

I wonder how hard it would be to have a way to turn on some validation to make sure that no templated vars go unfilled in the PDOM or alerts. I'm thinking that this would be something added onto while fuzzing or something. We may even be able to do something simple like (untested)
assert && assert( !/\{\{\w+\}\}/.test( PDOMContainer.innerHTML ), )
@jessegreenberg what do you think? It may be one (small) step in feel more confident about description refactorings/regression testing.
Questions:

When to enable this?

whenever assert is
assert slow?
only when fuzzing

What sort of impact might this have if we run this every frame?
We should test the PDOM and anything running through UtteranceQueue. Anything else?

This sounds like a very nice things to catch!

We should test the PDOM and anything running through UtteranceQueue. Anything else?

Would the regex catch attributes too? Just thinking things like aria-label and maybe others.

Would the regex catch attributes too? Just thinking things like aria-label and maybe others.

Yes it seems to.
I created this jsperf to test speed https://jsperf.com/speed-of-test/1
The test grabs the PDOM from gravity-force-lab-basics, and runs !/{{\w+}}/.test( PDOMContainer.innerHTML ). My laptop with chrome could run it 10635 times per second -> 0.00009402914 seconds per run -> 0.09402914 ms per run? For 17ms per frame, that is 0.5% of the time for the frame?
Hope I did that right. That makes me think putting behind assert is OK.

We can talk more at next dev meeting.

no templated vars go unfilled in the PDOM or alerts

What about taking it one step further and making it so that no templated vars show up in any Text or HTMLText or MultilineText or RichText?  When would we ever want a templated var to show up to the user in any fashion?

Maybe we make StringUtils.fillIn more strict to never allow template variables.

Maybe we make StringUtils.fillIn more strict to assert out if there are any remaining template vars after filling in.

@pixelzoom and I discussed that idea previously, but discovered there are places in our code where fillIn is necessarily called multiple times on different variables.

I see, thanks. Then maybe a function in StringUtils that does something like the regex recommended by @zepumph that can be used in functions where we set text.

@pixelzoom and I discussed that idea previously, but discovered there are places in our code where fillIn is necessarily called multiple times on different variables.

Perhaps similar to us putting an assertion in setInnerContent instead of fillIn, we could add this sort of check to RichText and Text. Those are the only visual text Types in scenery right?

A few other types are mentioned in https://github.com/phetsims/scenery/issues/964#issuecomment-480413625

HTMLText extends Text
MultilineText is deprecated. Even so if there was another Type we could add that validation logic there too.
Potential solution: use the same validator in all these locations:
validate( value, {   valueType: 'string',   isValidValue: v=> !/\{\{\w*\}\}/.test( v ) } );
which will assert out.
When I apply a patch I will soon provide, I get some really nice assertions. I really want to commit this! But we should sort out the issues first.
rutherford-scattering
Uncaught Error: Assertion failed: value failed isValidValue: Protons {{protons}}
Error: Assertion failed: value failed isValidValue: Protons {{protons}}
    at window.assertions.assertFunction (http://localhost/assert/js/assert.js:22:13)
    at Object.isValueValid (http://localhost/axon/js/ValidatorDef.js?bust=1554757430901:224:41)
    at validate (http://localhost/axon/js/validate.js?bust=1554757430901:28:20)
    at Object.setTextContent (http://localhost/scenery/js/accessibility/AccessibilityUtil.js?bust=1554757430901:316:7)
    at AccessiblePeer.setLabelSiblingContent (http://localhost/scenery/js/accessibility/AccessiblePeer.js?bust=1554757430901:754:25)
    at AccessiblePeer.update (http://localhost/scenery/js/accessibility/AccessiblePeer.js?bust=1554757430901:230:14)
    at AccessibleInstance.initializeAccessibleInstance (http://localhost/scenery/js/accessibility/AccessibleInstance.js?bust=1554757430901:135:19)
    at new AccessibleInstance (http://localhost/scenery/js/accessibility/AccessibleInstance.js?bust=1554757430901:55:10)
    at Function.createFromPool (http://localhost/phet-core/js/Poolable.js?bust=1554757430901:108:22)
    at Object.createTree (http://localhost/scenery/js/accessibility/AccessibilityTree.js?bust=1554757430901:367:41)

circuit-construction-kit-ac-and-dc
Uncaught Error: Assertion failed: value failed isValidValue: Animation speed limit reached! Simulation speed reduced to {{percent}}% normal.
Error: Assertion failed: value failed isValidValue: Animation speed limit reached! Simulation speed reduced to {{percent}}% normal.
    at window.assertions.assertFunction (http://localhost/assert/js/assert.js:22:13)
    at Object.isValueValid (http://localhost/axon/js/ValidatorDef.js?bust=1554757408735:224:41)
    at validate (http://localhost/axon/js/validate.js?bust=1554757408735:28:20)
    at ChargeSpeedThrottlingReadoutNode.setText (http://localhost/scenery/js/nodes/Text.js?bust=1554757408735:132:7)
    at ChargeSpeedThrottlingReadoutNode.set text [as text] (http://localhost/scenery/js/nodes/Text.js?bust=1554757408735:152:30)
    at http://localhost/scenery/js/nodes/Node.js?bust=1554757408735:4969:25
    at u (http://localhost/sherpa/lib/lodash-4.17.4.min.js?bust=1554757408735:5:432)
    at Function.ru (http://localhost/sherpa/lib/lodash-4.17.4.min.js?bust=1554757408735:67:656)
    at ChargeSpeedThrottlingReadoutNode.mutate (http://localhost/scenery/js/nodes/Node.js?bust=1554757408735:4956:9)
    at ChargeSpeedThrottlingReadoutNode.Node [as constructor] (http://localhost/scenery/js/nodes/Node.js?bust=1554757408735:526:12)

HTMLText extends Text
MultilineText is deprecated. Even so if there was another Type we could add that validation logic there too.

RichText

Yes RichText has been included in this discussion.
This patch has helped to find a few bugs as expected and demonstrated above, but it also found an edge case, which is how RichText supports links.
Error: Assertion failed: value failed isValidValue: New version available
    at window.assertions.assertFunction (http://localhost/assert/js/assert.js:22:13)
    at Object.isValueValid (http://localhost/axon/js/ValidatorDef.js?bust=1554758645905:227:41)
    at validate (http://localhost/axon/js/validate.js?bust=1554758645905:28:20)
    at RichText.setText (http://localhost/scenery/js/nodes/RichText.js?bust=1554758645905:713:7)
    at RichText.set text [as text] (http://localhost/scenery/js/nodes/RichText.js?bust=1554758645905:728:30)
    at http://localhost/scenery/js/nodes/Node.js?bust=1554758645905:4969:25
    at u (http://localhost/sherpa/lib/lodash-4.17.4.min.js?bust=1554758645905:5:432)
    at Function.ru (http://localhost/sherpa/lib/lodash-4.17.4.min.js?bust=1554758645905:67:656)
    at RichText.mutate (http://localhost/scenery/js/nodes/Node.js?bust=1554758645905:4956:9)
    at new RichText (http://localhost/scenery/js/nodes/RichText.js?bust=1554758645905:233:10)

Patch
Index: scenery/js/accessibility/AccessibilityUtil.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- scenery/js/accessibility/AccessibilityUtil.js	(revision 803d3ffd4ac75b784417e901d8fcc9f88bf26dd0)
+++ scenery/js/accessibility/AccessibilityUtil.js	(date<PHONE_NUMBER>471)
@@ -15,6 +15,7 @@
   'use strict';

   // modules
+  const validate = require( 'AXON/validate' );
   var AccessibleSiblingStyle = require( 'SCENERY/accessibility/AccessibleSiblingStyle' );
   var Random = require( 'DOT/Random' );
   var scenery = require( 'SCENERY/scenery' );
@@ -310,6 +311,12 @@
     setTextContent: function( domElement, textContent ) {
       assert && assert( domElement instanceof Element ); // parent to HTMLElement, to support other namespaces
       assert && assert( typeof textContent === 'string' );
+
+      validate( textContent, {
+        valueType: 'string',
+        isValidValue: v => !/\{\{\w*\}\}/.test( v )
+      } );
+
       if ( tagNameSupportsContent( domElement.tagName ) ) {

         // only returns true if content contains listed formatting tags
Index: scenery/js/nodes/RichText.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- scenery/js/nodes/RichText.js	(revision 803d3ffd4ac75b784417e901d8fcc9f88bf26dd0)
+++ scenery/js/nodes/RichText.js	(date<PHONE_NUMBER>044)
@@ -60,6 +60,7 @@
   'use strict';

   // modules
+  const validate = require( 'AXON/validate' );
   var ButtonListener = require( 'SCENERY/input/ButtonListener' );
   var Color = require( 'SCENERY/util/Color' );
   var extendDefined = require( 'PHET_CORE/extendDefined' );
@@ -708,6 +709,12 @@
       // cast it to a string (for numbers, etc., and do it before the change guard so we don't accidentally trigger on non-changed text)
       text = '' + text;

+
+      validate( text, {
+        valueType: 'string',
+        isValidValue: v=> !/\{\{\w*\}\}/.test( v )
+      } );
+
       if ( text !== this._text ) {
         var oldText = this._text;

Index: scenery/js/accessibility/Accessibility.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- scenery/js/accessibility/Accessibility.js	(revision 803d3ffd4ac75b784417e901d8fcc9f88bf26dd0)
+++ scenery/js/accessibility/Accessibility.js	(date<PHONE_NUMBER>587)
Index: scenery/js/nodes/Text.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- scenery/js/nodes/Text.js	(revision 803d3ffd4ac75b784417e901d8fcc9f88bf26dd0)
+++ scenery/js/nodes/Text.js	(date<PHONE_NUMBER>127)
@@ -12,6 +12,7 @@
   'use strict';

   // modules
+  const validate = require( 'AXON/validate' );
   var escapeHTML = require( 'PHET_CORE/escapeHTML' );
   var extendDefined = require( 'PHET_CORE/extendDefined' );
   var Font = require( 'SCENERY/util/Font' );
@@ -128,6 +129,11 @@
       // cast it to a string (for numbers, etc., and do it before the change guard so we don't accidentally trigger on non-changed text)
       text = '' + text;

+      validate( text, {
+        valueType: 'string',
+        isValidValue: v=> !/\{\{\w*\}\}/.test( v )
+      } );
+
       if ( text !== this._text ) {
         var oldText = this._text;
         this._text = text;
Index: axon/js/ValidatorDef.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- axon/js/ValidatorDef.js	(revision 2da6c6173a0ed6bedc1dad8a5fffac27faaf5259)
+++ axon/js/ValidatorDef.js	(date<PHONE_NUMBER>216)
@@ -15,7 +15,7 @@
  *
  * A validator that accepts any Object:
  * { valueType: Object }
- *
+ *
  * A validator that accepts Enumeration values:
  * { valueType: MyEnumeration }
  * or
@@ -221,6 +221,9 @@
         return false;
       }
       if ( validator.hasOwnProperty( 'isValidValue' ) && !validator.isValidValue( value ) ) {
+
+        debugger;
+
         assert && options.assertions && assert( false, `value failed isValidValue: ${value}` );
         return false;
       }

These three stacks are the only errors I have found with this patch so far (halfway through normal phet fuzz, and completed a11y fuzzing)

Note from @samreid and @chrisklus - Simula Rasa shows "{{TITLE}}" for the simulation title, so that will have to be excluded.

Note from @samreid and @chrisklus - Simula Rasa shows "{{TITLE}}" for the simulation title, so that will have to be excluded.

This in addition to the way that RichText handles links makes two edge cases on the visual view side that I am not quite sure about how to handle. In the above commit I added logic to exclude template vars from the PDOM. This was straightforward, and only found one case that needed fixing.
@samreid would you please review the implementation with two main thoughts in your mind:

How I added this specific validating function to the validate function? I like the usage of this, and I think it is well organized and clear what it does. It is nice because it can be reused by any other type in the future, like RichtText to validate the same thing. I do however this it is a bit strange to put a function onto another function, especially since validate is named lower case. I also like the idea of having this space to add more of these reusable validating functions if we see fit. Another spot this could go that I just thought of is in StringUtils.
How might we be able to add this functionality to the view side code while handling the cases mentioned above?

Thanks,
Michael

To me, it seems more idiomatic to factor out the validator like so:
// @public - Prevents leakage of string templates into user-facing strings.
ValidatorDef.VALIDATOR_STRING_WITHOUT_TEMPLATE_VARS = {
  valueType: 'string',
  isValidValue: v => !/\{\{\w*\}\}/.test( v )
};

Then usage would be like:
// Disallow any unfilled template variables to be set in the PDOM.
validate( textWithoutEmbeddingMarks, ValidatorDef.VALIDATOR_STRING_WITHOUT_TEMPLATE_VARS );

I cannot think of a way to ignore this detection for {{TITLE}} for Simula Rasa.  Some alternatives:

Change Simula Rasa to say "TITLE" instead of "{{TITLE}}".  I'm not sure if there are other places with this pattern or if they could also use this workaround.
Text, PDOMUtils.options, and RichText could have a textValidator key which defaults to ValidatorDef.VALIDATOR_STRING_WITHOUT_TEMPLATE_VARS, and a different one could be specified if necessary.  However, if we use that in Simula Rasa, we would also want to strip it out on template instantiation.

I like the refactor to ValidatorDef, thanks!
I do not think that we should spend any more energy on this validation. In the PDOM, this is really valuable because it can be quite challenging to view all possible strings, especially for aria-live alerts that only occur in specific interaction context.
For the visuals, the complexity is much less, and I feel good about leaving it along. In general it is obvious when a templated string has not been filled in. Closing, please reopen if you want to discuss more.


Update ESLint

ESLint >= 9 brings a few breaking changes, making the dependabot PRs
that try to update it fail; cf. #5824, #5819, #5813, #5812.
The main thing was a change in the default configuration format.
There is a compatibility flag, but I don't think we want to use it,
since the old way is also deprecated now. See also
https://eslint.org/docs/latest/use/migrate-to-9.0.0#-new-default-config-format-eslintconfigjs.
Without that, updating basically entailed the following:

The shared config in docs/checkstyle was converted to the new
format,
making it a full npm-package in the process.

This is required because of the way the new format handles plugins
and other dependencies.
Note that this also makes the resolve-plugins-relative-to option
obsolete,
which is good since it is no longer supported.
This patch removes it everywhere we used it.
I also added a dependabot rule to keep that package's few dependencies
up to date.

The --config option can't load such shared configs, though,
so all the packages that use this combination got a new config
of their own, basically deferring to that package.
Modules which already had their own config file had it updated as
well.
.eslintignore is no longer supported and thus got moved to
the new config file(s).
I also removed some unnecessary eslint-disable comments.

The eslint-plugin-header is no longer compatible with ESLint 9.
I replaced it with eslint-plugin-headers, but that required
kind of an ugly hack.
See also https://github.com/robmisasi/eslint-plugin-headers/issues/4.
Note that this udpate might slightly change the linting rules;
I did not check every last change to default rules and options!
The one thing I hade to change was to add a setting to the no-unused-vars-rule,
because the corresponding default changed, and we depend
on the old default.
See also https://eslint.org/docs/latest/use/migrate-to-9.0.0#-no-unused-vars-now-defaults-caughterrors-to-all.
Note also that the lti-module also uses ESLint, but as part of it
being a create-react-app, so we don't control the ESLint version
in there. I didn't touch that module. (Well, in this PR; see #5830.)
We might want to change that in the future, though, since CRA
is deprecated in itself.
Another thing we might want to change in the future is
the engage-paella-player-7-module using the deprecated
@babel/eslint-parser.
We also use a few deprecated ESLint rules, but for now I wanted
to chagne the configs as little as possible, since the necessary
changes are already quite sweeping. I might make a follow-up PR
in the coming days.

OK so, CI fails here, because I failed to remember that installing local packages only symlinks them, so the target folder needs to have its dependencies already installed. I think there is a fix and I will try and push it in a second here, but this kind of raises the question of whether we shouldn't properly publish that shared config somewhere, be it on npm proper, or here on GitHub somehow.
And that in turn raises the question of how this relates to the ESLint-config we already published…


Make request password reset email case-insensitive

Summary
When a bank requests a password reset for a partner user,  the check for whether that user exists for that partner is case sensitive.  It shouldn't be.
Details
To see the problem in action on your local,

Sign in as<EMAIL_ADDRESS>Click on Partner Agencies | All Partners
Click on one of the partners
Scroll down to find  the email of  a User who can access this Partner
Click on "Reset Partner Password" (near the top)
put in the email,  but differently cased (example<EMAIL_ADDRESS>instead of<EMAIL_ADDRESS>
It will tell you: Could not find partner user for this partner with email [Email  you actually entered]!
It should match in a case-insensitive manner.
Criteria for completion

[ ] using an email that is the same as one of the users on the partner, except that the case of the letters is not the same works
[ ] the functionality of the bank's reset of partner passwords is otherwise unchanged
[ ] test to support the above

I can take that!


[DOWNSTREAM-ONLY] cleanup: address MarkDown linter issues

The super-linter/super-linter/slim action reports errors related to
the MarkDown templates and README. These files are not part of the
upstream project, so the corrections are downstream only.
Signed-off-by: Niels de Vos<EMAIL_ADDRESS>

/lgtm


tidy up for going live

This is by no means finished; this is just a first stab at making the site presentable and removing entirely blank pages for now.
Ready for review.

done

One comment, otherwise ready to merge.

ok cleaned up the commit hx and addressed comments, @bqpd please merge if happy


SCSS Is classified as CSS in the language bar, but not in the search feature

Problem Description
SCSS files are classified as being CSS, but when clicked, there are no search results

URL of the affected repository: https://github.com/SMLHelper/SMLHelper.github.io

This is a side effect of the Language grouping that happens within Linguist but not differentiated by search.
SCSS is part of the CSS group:
https://github.com/github/linguist/blob/1221480189454d80e02f3811612f79e2aadd8f91/lib/linguist/languages.yml#L4191-L4194

@lildude might be time to fix that one on GitHub.com's side...?

So this is working as intended?

SCSS is part of the CSS group

This has always irked me.
SCSS has less in common with CSS than CoffeeScript does with JavaScript. I know this has been discussed before, time and again, but as someone with a background in front-end web development, I can honestly tell you that classifying SCSS as CSS looks very jarring.

@lildude might be time to fix that one on GitHub.com's side...?

Possibly, though I'm not part of the team that controls search and how it behaves so have little influence on the behaviours or product direction. I also don't know how it works on that side of thing 😄

@AlexejheroYTB you removed your comment, but I saw it 😉... yes, it is working as intended.

What about on the github side?

What about on the github side?

Yup. It's working as intended too.

"No files found" ?

It's returning no files because it is searching for the language CSS as you can see from the URL (you pointed this out in your first screenshot):
https://github.com/SMLHelper/SMLHelper.github.io/search?l=css   <---- LOOKY HERE

As you pointed out, your repo doesn't have any CSS files, hence no results. Change css to scss and 🎉 , files as shown in your third screenshot.
The limitation is search doesn't have a concept of searching for a language group, only individual languages.

Every limitation can be overcome ;)

This might be confusing for some users, thats why I asked

Should I close this now, or wait for a "fix"?

Best to close. This isn't on anyone's roadmap right now and isn't likely to change on the GitHub side of things anytime soon. When it does, Linguist will certainly be updated to accommodate, if necessary.


Please talk more about the benefits

I can see that GVFS can be very useful in a big repo. It's not that valuable for small repoes.
So when do I really need this? How big should my repo become to use this? How good is this?

That is a great question, and one that we will work to give more specific guidance on. Some general thoughts...
You're absolutely right that there's no need to virtualize smaller repos. In general, if you're going to need most/all of the contents of a repo, doing a bulk copy of all of the contents is MUCH faster than copying each file one at a time. So even for larger repos, the same holds: if you're going to use all of the files, e.g. because you will build from the root and the build touches everything, then again you lose much of the benefits of virtualizing.
Based on our own experience within Microsoft, I can tell you that if your codebase is roughly 100K files or fewer, and less than single digit GBs of content, Git works pretty well as is, especially with some of the more recent perf improvements that have been made there.
If your codebase is 1M files or more, then chances are that you have a sophisticated build infrastructure as well, and that most of your devs don't build all of the sources themselves. The most common workflow on teams of that size is that devs download the build outputs from the most recent official build, and then run an incremental build on some subfolder that contains the sources they are working in.
If you fit both of those conditions: 1) You have lots of source files (I'd guess 250K or more? 500K or more?) and 2) devs don't need to access every file in their day to day work; then you might benefit from virtualizing.
GVFS provides two important functions: one is that it helps you avoid downloading file contents that you don't need. The other is that it manages the specific set of files that Git has to consider when you run commands like "git status", "git checkout", etc. So we're tackling both the problem of size, but also the problem of too many files.


Move API from v1alpha1 to v1.

This only moves the types as they are currently to the new version. We
will begin updating for V1 shortly.
Likewise we will soon restore v1alpha1 types and the migration code +
aggregated APIserver to handle them.

I've created a v1 branch off master, and this PR to merge the first work into that branch, rather than master.

My bad, I think I've got them all now.
$ git grep v1alpha1 | grep -v vendor | grep -v Gopkg.lock                  (hive/172-17-0-4:6443//hive)
cmd/manager/main.go:	crv1alpha1 "k8s.io/cluster-registry/pkg/apis/clusterregistry/v1alpha1"
cmd/manager/main.go:			if err := crv1alpha1.AddToScheme(mgr.GetScheme()); err != nil {
config/templates/hive-csv-template.yaml:apiVersion: operators.coreos.com/v1alpha1
hack/olm-registry/olm-artifacts-template.yaml:- apiVersion: operators.coreos.com/v1alpha1
hack/olm-registry/olm-artifacts-template.yaml:- apiVersion: operators.coreos.com/v1alpha1

make verify and make test are clean.


Update golang.org/x/tools/... packages in //third_party. [phab:D244]

Created by fromberger at 2015-05-13 14:37:23:

Updates from upstream, plus a missing CAMPFIRE rule.

fromberger wrote at 2015-05-13 21:11:41:
Address review comments.

schroederc commented inline (context lost during migration) at 2015-05-13 21:13:28:
The "third_party/go/" prefix should be removed on each source file.

schroederc required changes at 2015-05-13 21:13:28

fromberger wrote at 2015-05-13 21:16:39:

Fix source paths to be correctly relative to the package.

fromberger wrote at 2015-05-13 21:23:22:

Add go_build property to the go_types rule.

schroederc accepted this revision at 2015-05-13 21:24:01

schroederc commented inline (context lost during migration) at 2015-05-13 21:42:05:
Can you remove this file from the srcs please?

fromberger wrote at 2015-05-13 21:45:19:

Drop test file from its library target.

fromberger wrote at 2015-05-13 22:07:08:
Rebase from upstream.

fromberger closed via commit b97d258ecb4d12f7577f368bd535654c7307b565 at 2015-05-13 22:10:32


Fixed #32210 -- Default value of UUIDField in Inline Formset

ticket-32210

Maybe you can add a unit test that reproduces the bug stated in the ticket.

@sainipray I think we need a fresh test more clearly showing the issue, rather than just an edit on the existing test that failed, don't you? 🤔

Maybe you can add a unit test that reproduces the bug stated in the ticket.

@sainipray I think we need a fresh test more clearly showing the issue, rather than just an edit on the existing test that failed, don't you? 🤔

Yes, I'll try to add a test case.
but I checked the ticket https://code.djangoproject.com/ticket/24958. and commit for this https://github.com/django/django/commit/a50b66da30320887c23c73927f6b2ab41e0301bf
so that commit added some code there but caused this issue (https://code.djangoproject.com/ticket/32210), I think that case I edit should not be there according to changes.

@sainipray If you think it should be removed, or adjusted as you have, please do so. (We can decide at the end)
Ideally, an extra case that would have failed preventing the regression would be awesome.
(You're looking at this, so you're the current expert 🙂)

Thanks for the quick update @sainipray — I will give it another look tomorrow. 👍

Hi @sainipray.
Sorry for the slow follow-up here: I'm still deeply suspicious about the change here — I think we need further test cases to show that it's correct.
This was the case before a50b66d for ticket-24958:
if self.instance._state.adding and form._meta.model._meta.pk.has_default():
    self.instance.pk = None

With the change here, we would have:
if self.instance._state.adding:
    to_field = self.instance._meta.pk
    if to_field.has_default():
        setattr(self.instance, to_field.attname, None)

Bar the reformatting it's exactly the same logic. As such, the effect is to revert a50b66d — but just inverting the assertion in the test case doesn't show we didn't introduce a regression. 🤔 (If we're saying a50b66d was wrong then some account of that would help see the conclusion.)
Combined with the semantics of ignoring the to_field I'm still not convinced this is correct.
Taking the example project from the ticket, can we examine the value of form.instance.id (the to_field in that case) coming out of BaseInlineFormSet._construct_form() — is that correct.
I'm still looking, but I thought I'd comment in the meantime in case you can make progress with additional tests/more diagnosis.
Thanks!

Hi @carltongibson, I'm ignoring to_field because it can store the parent model field name that have relation to the child model.
Suppose we have two models like the bottom:
class ParentWithUUIDAlternateKey(models.Model):
    uuid = models.UUIDField(unique=True, default=uuid.uuid4, editable=False)
    name = models.CharField(max_length=50)

class ChildRelatedViaAK(models.Model):
    name = models.CharField(max_length=255)
    parent = models.ForeignKey(
        ParentWithUUIDAlternateKey, models.CASCADE, to_field="uuid"
    )

Here uuid field have a relation with the child model field parent field as ForeignKey field and this uuid field is not primary_key which means we have id field as primary_key in the Parent model.
so when we save ParentWithUUIDAlternateKey data from Django admin with inlineformset_factory of model ChildRelatedViaAK.
kwargs['to_field'] = self.fk.remote_field.field_name
Here to_field value is uuid field
if kwargs.get('to_field') is not None:
    to_field = self.instance._meta.get_field(kwargs['to_field'])
else:
    to_field = self.instance._meta.pk

So In if in condition, to_field value become is uuid field
if to_field.has_default():
      setattr(self.instance, to_field.attname, None)

and now this code reset uuid field value to None
so when this uuid field actually saving in the database, Database returns IntegrityError.
We only regenerate the value of the primary_key field here, so we should only reset value of primay_key.
that's why I consider pk field and skip to use 'to_field' from kwargs.
You mentioned this code
if self.instance._state.adding and form._meta.model._meta.pk.has_default():
    self.instance.pk = None

this is the same as I did. We can use this as well.
Let me know if you have still any doubts.

Hi @carltongibson Any update about this PR?

@sainipray I still haven't seen why it is that we're simply reverting https://github.com/django/django/commit/a50b66da30320887c23c73927f6b2ab41e0301bf — I flagged Needs tests on the ticket because I think we need to show that all cases are correct.
Returning to ticket-24958, can we show that there's not a regression there with this change? (It's not enough to just invert the assertion, and then say look it passes — we need to see that's correct, and I'm perhaps slow, but it's not obvious to me that it is.)
I haven't had a chance to read through your last comment to see if that makes it clear.

I tested in the admin using this setup (models from the test suite):
from django.contrib import admin

from .models import ParentWithUUIDAlternateKey, ChildRelatedViaAK

class Inline(admin.StackedInline):
    model = ChildRelatedViaAK

admin.site.register(ParentWithUUIDAlternateKey, inlines=[Inline])

Without this patch, I can reproduce the integrity error when trying to add a new instance with inlines. When trying the same with this patch, I get "Please correct the errors below." with no other details in the UI. Printing the formset errors in ModelAdmin._changeform_view() shows: ["[{'parent': ['The inline value did not match the parent instance.']}, {'parent': ['The inline value did not match the parent instance.']}, {}]"]. It seems further investigation is needed.

Closing due to inactivity.


identity: Fix nil pointer panic in LookupIdentityByID

Because the identity allocator is initialized asynchronously via
InitIdentityAllocator, its internal local identitiy allocator might
not have been initialized yet when the lookup functions are called.
This can cause nil pointer panics, as observed in #13479.
Before b194612c004c3e69289286e9a35d337b2645fc50, this nil pointer panic could not occur in
LookupIdentityByID as the function checked for m.IdentityAllocator != nil
which also implies m.localIdentities != nil.
This commit adds an explict check for m.localIdentities.
Fixes: #13479
Fixes: b194612c004c ("identity: Avoid kvstore lookup for local identities")
Signed-off-by: Sebastian Wicki<EMAIL_ADDRESS>
Note: While the backported version of b194612c004c in v1.7 and v1.8 does not suffer from the nil pointer panic due to an early check fo m.IdentityAllocator != nil , I think we should still back-port this to fix the potential nil-pointer panic in LookupIdentity which is also fixed by this commit.

test-me-please

@gandro why don't we need to wait before the variable is assigned with the <-m.localIdentityAllocatorInitialized?

Fair point. My thinking was that I would restore the behavior present in 1.7/1.8. But assuming there are no issues with deadlocks, I guess we could rely on m.localIdentityAllocatorInitialized instead. I'll update the PR.

@gandro why don't we need to wait before the variable is assigned with the <-m.localIdentityAllocatorInitialized?

Fair point. My thinking was that I would restore the behavior present in 1.7/1.8. But assuming there are no issues with deadlocks, I guess we could rely on m.localIdentityAllocatorInitialized instead. I'll update the PR.
Edit: Overall, I don't think waiting on m.localIdentityAllocatorInitialized will change anything however. Once it becomes ready, the local identity cache will be empty, so it will also return simply nil.

@gandro the race detector will complain that we are accessing the variable concurrently :smile:

@aanm PTAL again. After looking a bit more closely at initialization, I decided to do a non-blocking check on the channel. This ensures that we have proper synchronization point to avoid data races. In the process, I found some additional potential data races and nil pointer panics that I fixed.
I decided to fail the lookup instead of waiting for the caches to be initialized. If we wait for full initialization including synchronization, then both the DNS Proxy (call site) and Hubble (call site) would not be operational until we have synchronized the identity cache. I don't think we want that.
If callers need to perform a lookup on a synchronized cache, they should call WaitForInitialGlobalIdentities first.

test-me-please

GKE hit a flake "could not get service LoadBalancer IP addr: 30s timeout expired" https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/2802/
This does not seem related

test-gke

Hit #13554 twice

Rebased to pick up the fix the the GKE flake this observed. Rerunning CI

test-me-please

netnext hit #13261
https://jenkins.cilium.io/job/Cilium-PR-K8s-1.12-net-next/866/testReport/junit/Suite-k8s-1/12/K8sServicesTest_Checks_service_across_nodes_Tests_NodePort_BPF_Tests_with_direct_routing_Tests_NodePort_with_Maglev_Tests_NodePort/

retest-net-next

Potentially new net-next flake? https://jenkins.cilium.io/job/Cilium-PR-K8s-1.12-net-next/872/testReport/junit/Suite-k8s-1/12/K8sServicesTest_Checks_service_across_nodes_Tests_NodePort_BPF_Tests_with_direct_routing_Tests_NodePort_with_Maglev_Tests_NodePort/

retest-net-next


Create hooks for Focus Trap and  ESC listener and use in Modal, Popover and Tooltip

Scope of this change:

Extract out a hook for the ESC handler
Extract out a hook for focus trapping
Use them in Popover, Tooltip and ModalCurtain
Convert ModalCurtain to a function component with hooks in order to allow it to use the new hooks
Extract out some Tooltip logic into a function component to use the hooks.

Not doing:

The main Tooltip is still a class – converting it to be a function component should be another PR, since it has a lot of lifecycle hooks and state, and we want to make sure we don't break anything. When we do this, Tooltip can use the hook directly.

Converting the deprecated ModalStandard and it's underlying ModalBase to use these hooks. These components have their own ESC handling, but it's not worth the effort to rewrite them as function components. We should work on removing uses in website and deleting them.

I'm getting this on Chrome when clicking on the button to open the popover.
Can you either:

Remove the outline from the outer container? (Normally I'm against removing outlines, but the outermost element is not actually interactive and when the user hits Tab they'll see a focus state.)
Work with Jon to restyle it?

I'd personally prefer the first option but am open the second if you feel strongly.

removed the outline


NHMV volumes are missing preview images

With the recent update to the NH holdings, some of the NHMV images do not have preview images for OPUS.

I believe this was fixed by @markshowalter 's recent update of holdings. If the next OPUS import is still missing images, I will reopen this issue.


tools: Add fap-lld wrapping linker that generates .fast.rel sections

Closes flipperzero-rs/flipperzero#89.

In local testing on my Macbook Air M1, this reduces cargo test runtimes from 13s to 11.7s.

From the error in the example build logs, it looks like we can't use whatever objcopy is in PATH, and will probably need to just use llvm-objcopy and tell people to rustup component add llvm-tools to get FastFAP support.

Rebased on main to fix merge conflicts caused by #97 being merged (which I had been holding off merging, because dependabot will perform these kinds of rebases automatically on its own PRs).

Force-pushed to use deterministic filenames for the .fast.rel section data.


[AutoDiff upstream] Update gyb-generated files.

Update gyb-generated files for @differentiable attribute.
Friend PR: https://github.com/apple/swift/pull/27506

This fixes CI breakages after merging https://github.com/apple/swift/pull/27506: https://ci.swift.org/job/swift-PR-osx/16810/console.
We'll be sure to run @swift-ci Please test to catch such errors (--swiftsyntax-verify-generated-files) on future PRs.

https://github.com/apple/swift/pull/27506
@swift-ci Please test

Build failed
Swift Test OS X Platform
Git Sha - 6d2c80e04ce91ef4e9351599070d8a58eb58c009

@swift-ci Please test


[doc] D2 checklist updates

This updates the D2 checklist with several links to scripts and GitHub issues, and adds a D2 checklist item for the SecRnd parameters (https://github.com/lowRISC/opentitan/issues/2229).

Rebased with suggested changes applied.


ggshield pre-push scans more commits than necessary

GitGuardian Shield Version

1.12.0

Describe the bug
When scanning a new branch, ggshield pre-push hook is not able to detect the branch start and resort to scanning the last N commits (N being 50 by default).
Expected behavior
The hook should only scan the commits between the branch start and the HEAD.
pre-commit supports this (see https://github.com/pre-commit/pre-commit/blob/bdc08d828591b90a3371f6abbaec325a0f36b79d/pre_commit/commands/hook_impl.py#L134). ggshield should do something similar.
TODO

[ ] Fix the bug for pre-push
[ ] See if the fix can also be applied to pre-receive. If it's simple: fix it. If it's not, open an issue to fix this on pre-receive.

The attached script (repro-git-push-new-branch.zip) makes it easy to reproduce the bug.

Can you also test this use case on tagging as well


Release/v0.3.2

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@a42e932). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #19   +/-   ##
=======================================
  Coverage        ?   50.38%
=======================================
  Files           ?       14
  Lines           ?      387
  Branches        ?        0
=======================================
  Hits            ?      195
  Misses          ?      192
  Partials        ?        0

Continue to review full report at Codecov.

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


AGM is way too slow!

AGM is pretty slow, when you have 500+ Markers. There are pretty much usecases, where this is a normal amount. In my current project, there are many thousands. (Of course, I use the MapClusterer).
Some Numbers (similar code as in tutorial):

AGM Page loads in 40+ seconds.
if I remove the  (demo content), it's still more than the half seconds.
my old JQuery-Implementation renders same content / data with info-windows and Clusterer within 6.x seconds.

reproduce by modifying plunkr-code something like:
markers: marker[];
  for(i = num;i>=1000;i--) {
   markers.push({
    {
		  lat: 51.673858,
		  lng: 7.815982,
		  label: 'A',
		  draggable: true
	  }
   });
  }

(maybe fill values with randoms)
My current solution is to "iframe" the content of my old jquery-implementation. Not a beautiful solution, but a fast workaround.
Most answers I found to this issue were like '500+ is too much...' -> troll-postings. Even 5000+ can easily be handled by a simple gmap + jquery implementation.
"@agm/core": "^1.0.0-beta.2",
"@agm/js-marker-clusterer": "^1.0.0-beta.2",

@nylz I had the same issue trying to render 1000+ points. I switched to ChangeDetectionStrategy.OnPush and that really helped. See https://github.com/SebastianM/angular-google-maps/issues/520#issuecomment-242485390. I can now render all these points (with clusters and info windows) in about 5 - 6 seconds.
However, I still had the issue when subsequently filtering this list down to 150 or even 20 items. You can see https://github.com/SebastianM/angular-google-maps/issues/1285 for more info. I'm am about to try to implement the solution proposed there.

Hey, thank you very much, for that hint. Now it is even faster, than my jQuery-implementation. Im pretty new to Angular, so I first had to click through this presentation: https://pascalprecht.github.io/slides/angular-2-change-detection-explained/#/
to understand, what the real problem here was.
What I did now:
changeDetection: ChangeDetectionStrategy.OnPush ... constructor(... ... private cd: ChangeDetectorRef) { ... onGotResponseFromMyService(resObj: MyResponse) {   // enrich component with received data   // and ... finally:   this.cd.detectChanges(); }
Imho, the name OnPush is a bit confusing. Would be easier to understand if it was labeled ChangeDetectionStrategy.Manually
however. Thank You for support.

@nylz I had the same issue trying to render 1000+ points. I switched to ChangeDetectionStrategy.OnPush and that really helped. See #520 (comment). I can now render all these points (with clusters and info windows) in about 5 - 6 seconds.
However, I still had the issue when subsequently filtering this list down to 150 or even 20 items [Solved!]. You can see #1285 for more info. I've post my solution and there's another example there as well.

@aarontoys thanks for the magic trick ! It worked like a charm actually

I had the same issue. I had AGM map with 3000+ markers integrated in my project. It's became very slow, as a solution added change detection strategy onpush on app component as declaring on map component didn't improve performance much. Is there any better solution for this. For all these markers we are having agm-info-window with bunch of data. How to improve agm map performance with high nos of markers.

@scode only display markers that are inside the bounds

have u guys found the solution?


 For example, if an scheduled export fails because of some problem uploading the files to Google Drive, the user won't notice until he or she checks for the exported files. This may happen too late, when the user is looking for a backup to recover from a data loss.
The scheduled actions service should register any error so the user is informed the next time the application is run.

The idea I have in mind is to add two new fields to scheduled_actions table: error_count and error_message.
error_count stores how many times a scheduled action has failed. It's reset to 0 when the action succeeds or when the user is informed.
error_message stores information about the last time the action failed.
The next time the user runs the application, it checks if any scheduled actions has failed. If any error has been registered, a Snackbar is shown informing of it. It would have an action to show the details of the actions that have failed and the error messages.

@rivaldi8 you mean adding that to the db table? I have reservations about this.
The way Android is designed, we are not responsible for what happens outside of the app. Once we hand over a file, we should not care how it is handled.
We only have issues now because my Google Drive integration is a bit buggy. I am working to switch this to the storage access framework in v2.2 which should make the export very very reliable. Eventually, we would phase out our own sync implementations for API levels >= 19 and let the users use the app of their favourite service with SAF support.
Also, I would prefer not to modify the db with extra fields not present in the GnuCash schema, because if we later on want to sync the db files, that would be another thing to handle.
We should just ensure that backups always work reliably before exports so that users can avoid data loss. One way would be to create a UI which offers the user a chance to undo the last delete. Then we can transparently restore from that backup for the user in case they want to. The UI can either be a SnackBar or an option in Settings. What say you?

Oh! I didn't remember you were working on switching to SAF. That would make this issue invalid then. I'll close it.
About the undo feature, I think we should have both the SnackBar and the option in Settings. The SnackBar showing up after some destructive actions, like "delete after export". The option in Settings for any time the user needs it. However, now that I think, there's an option already under "Backup & export". Do you mean to implement it differently?

This message was created automatically by mail delivery software.
A message that you sent could not be delivered to one or more of its
recipients. This is a temporary error. The following address(es) deferred:
<EMAIL_ADDRESS>Domain ownyourbits.com has exceeded the max emails per hour (3/3 (100%)) allowed.  Message will be reattempted later
------- This is a copy of the message, including all the headers. ------
Received: from o11.sgmail.github.com ([<IP_ADDRESS>]:13242)
by skywalker.onesite.co with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128)
(Exim 4.87)
(envelope-from<EMAIL_ADDRESS>id 1cV1QH-001wiQ-Tb
for<EMAIL_ADDRESS>Sat, 21 Jan 2017 14:34:02 -0500
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=github.com;
h=from:reply-to:to:cc:in-reply-to:references:subject:mime-version:content-type:content-transfer-encoding:list-id:list-archive:list-post:list-unsubscribe;
s=s20150108; bh=9zPRlxd+PTnDJXU/TPMB83he9dI=; b=teG7SDIV0xe56nT0
FkzWsxZOX9o8aSV19sChz4DEEfNt0XwxUrI5yB1KB/8cZLYeU+jfZcUjWu18ZUoB
8iK4e6UvRWYXHcFGhPUXCdQbQ/KMcCQqBL0iNBSDC1oPrOnO5Mc4/bghA5aXsJJl
dc44AAgAQO6Kw+CTJTnvmdY5H3w=
Received: by filter0444p1mdw1.sendgrid.net with SMTP id filter0444p1mdw1-11103-5883B781-D
2017-01-21 19:33:21.464507197 +0000 UTC
Received: from github-smtp2a-ext-cp1-prd.iad.github.net (github-smtp2a-ext-cp1-prd.iad.github.net [<IP_ADDRESS>])
by ismtpd0001p1iad1.sendgrid.net (SG) with ESMTP id CZ3lmSXxTGC5OWFu43j0bw
for<EMAIL_ADDRESS>Sat, 21 Jan 2017 19:33:21.389 +0000 (UTC)
Date: Sat, 21 Jan 2017 11:33:21 -0800
From: Àlex Magaz Graça<EMAIL_ADDRESS>Reply-To: codinguser/gnucash-android<EMAIL_ADDRESS>To: codinguser/gnucash-android<EMAIL_ADDRESS>Cc: Subscribed<EMAIL_ADDRESS>Message-ID<EMAIL_ADDRESS>In-Reply-To<EMAIL_ADDRESS>References<EMAIL_ADDRESS>Subject: Re: [codinguser/gnucash-android] Inform the user when a scheduled
action fails (#643)
Mime-Version: 1.0
Content-Type: multipart/alternative;
boundary="--==_mimepart_5883b7814bdba_4a8c3fa203ed313826775f";
charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: rivaldi8
X-GitHub-Recipient: nachoparker
X-GitHub-Reason: subscribed
List-ID: codinguser/gnucash-android <gnucash-android.codinguser.github.com>
List-Archive: https://github.com/codinguser/gnucash-android
List-Post<EMAIL_ADDRESS>List-Unsubscribe<EMAIL_ADDRESS>https://github.com/notifications/unsubscribe/AUWsXISF_z_4Adm-EqKfQo-vCLzXaPsrks5rUl2BgaJpZM4LqG_9
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address<EMAIL_ADDRESS>X-SG-EID: bZZJyvIOioDBIv7or3Raig/qRxFLJCSVN5+R3lL3f3jVZRgyTODYbDDCrduTA+skCvsgOayTPRiuIJ
OQq81BEKMsjvPwjO9h8dJYGyttt8rXUvdFvDjWMJAvDzCJ7alYPz/n2Pj9ZdzjB/zimzsx/MCKJboB
5QO/NdxotxSwnZ9HTnmQrv1F1MI63fQY0p3GO4KKXgrqcGZlOgsMLhbQ2iSnNoF3/N+GwYNGoKjD/W
WvK6hUcJ1iUB6VA88ULA36
----==_mimepart_5883b7814bdba_4a8c3fa203ed313826775f
Content-Type: text/plain;
charset=UTF-8
Content-Transfer-Encoding: 7bit
Oh! I didn't remember you were working on switching to SAF. That would make this issue invalid then. I'll close it.
About the undo feature, I think we should have both the SnackBar and the option in Settings. The SnackBar showing up after some destructive actions, like "delete after export". The option in Settings for any time the user needs it. However, now that I think, there's an option already under "Backup & export". Do you mean to implement it differently?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/codinguser/gnucash-android/issues/643#issuecomment-274282955
----==_mimepart_5883b7814bdba_4a8c3fa203ed313826775f
Content-Type: text/html;
charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Oh! I didn't remember you were working on switching to SAF. That would m=
ake this issue invalid then. I'll close it.
About the undo feature, I think we should have both the SnackBar and the=
 option in Settings. The SnackBar showing up after some destructive actions=
, like "delete after export". The option in Settings for any time the user =
needs it. However, now that I think, there's an option already under "Backu=
p & export". Do you mean to implement it differently?
&mda=
sh;You are receiving this because you are subscribed to this thread.<=
br />Reply to this email directly, view it on GitHub, =
or mute the thread.

=
----==_mimepart_5883b7814bdba_4a8c3fa203ed313826775f--

@rivaldi8 I agree that we should have both.
The current option in Settings makes the user select the file by date. This is easy but not the most convenient thing to do. We can leave that in place.
But the undo option (via SnackBar) would definitely be a great addition. It which will allow the user option to immediately restore the deleted transactions without going through any further dialogs.
A temporary backup file will be generated (which doesn't even need to go into the backups folder), it can just be a short-lived file created for the express purpose of doing an undo. When SnackBar is dismissed, the file is also deleted.
Something like that is what I was thinking about.

:+1: I like the idea.


Hotlinking: Fix CORS Proxy issue and link validation bug

Context

There was an issue with the CORS proxy not returning the correct response code, as well as an issue with URLs not being encoded before validated on the client-side.
Also, insertion when using the proxy was potentially quite slow, which was not very clear to users.
Summary

This PR contains a few (significant) changes to improve the overall UX and performance of hotlinking.

Accessibility: The hotlinking dialog can now be submitted by pressing enter.
User Feedback: The "Submit" button is now disabled while insertion happens.
A spinner could perhaps be added in the future.
Performance: Overhaul insertion logic to reduce the number of times the video is fetched.
Mostly relevant when loading larger video files via the CORS proxy.

Regarding the latter, a video is now preloaded only once, so that dimensions, length, and audio information can be retrieved.
If the user can upload media, the video is seeked to the initial frame to generate a poster image, even before the video is actually inserted.
Doing that before insertion has two main reasons:

Doing it after and updating the video using updateElementsByResourceId would cause an unexpected history entry.
Using updateElementsByResourceId for that would also mean the video gets re-rendered, which means playback stops.

Relevant Technical Choices

Refactored a lot of the logic for preloading videos and extracting metadata, to reduce repetition in multiple areas.
To-do

N/A
User-facing changes

The hotlinking dialog can now be submitted by pressing enter
The "Submit" button is now disabled while insertion happens

Testing Instructions

[x] This is a non-user-facing change and requires no QA

Proper QA will be split into its own ticket so it can be done more holistically.
Reviews
Does this PR have a security-related impact?

No
Does this PR change what data or activity we track or use?

No
Does this PR have a legal-related impact?

No
Checklist

[x] This PR addresses an existing issue and I have linked this PR to it in ZenHub
[x] I have tested this code to the best of my abilities
[x] I have verified accessibility to the best of my abilities (docs)
[x] I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
[x] This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
[x] I have added documentation where necessary
[x] I have added a matching Type: XYZ label to the PR

Fixes #9696

https://user-images.githubusercontent.com/237508/141507013-60778fc8-9908-48ad-b3b2-58c7f8c8a077.mov
This fix does not address this looping issue. https://sample-videos.com/video123/mp4/720/big_buck_bunny_720p_30mb.mp4

@swissspidy A hotlinking-related Karma change was merged meanwhile, that might fix the failing Karma test.

I think we can merge this change, as it does fix the issue and work well.
We can create another issue / PR for the looping issue.

Is that a valid karma test failure?

Is that a valid karma test failure?

Hm, it seems so, looks like the behaviour changed within this PR. Previously it just showed Invalid link when inserting d (used in the tests) as the link to insert, but now it seems to initially display a message about incorrect file extension instead which fails the Karma test. 🤔 The message changes to "Invalid link" after the link is passed to the endpoint for validation but Karma test catches the initial message.
It seems that isValidUrl is generally working a bit oddly -- it shouldn't probably consider https://d a valid URL even when encoded as this PR is now doing, no?

It seems that isValidUrl is generally working a bit oddly -- it shouldn't probably consider https://d a valid URL even when encoded as this PR is now doing, no?

Technically that's a valid URL (run new URL('https://d') to confirm). So valid-url correctly allows it.
For the sake of hotlinking we could add some stricter check on top of that though, for example disallow empty paths ("/") like we already do on the server side. After all without a path we don't have a valid file name.
I'll take a look.

@spacedmonkey @miina Re-requesting review because I ended up doing a bigger overhaul than originally anticipated.
I tried to slightly improve UX and also performance by only creating one <video> element to get all the metadata we need for a video. If only inserting media with reasonable size these two points shouldn't really be a concern though (happy case).
I'll leave holistic QA for hotlinking of various media types with different file sizes etc. to a dedicated QA ticket that I have yet to create.
Ideally we merge this PR as-is to unblock other ongoing work and — most importantly — unbreak the CORS proxy.

Inserting needs to be reset on closing the dialog.

https://user-images.githubusercontent.com/237508/142442587-fcf8dee2-487c-4446-83e3-be12a0862e30.mov

Inserting needs to be reset on closing the dialog.

Fixed now

Audio detection is broken in this PR now. You need to ensure that you seek the video first before doing the check.

🤔
Seeking is already happening:
https://github.com/google/web-stories-wp/blob/87c840f7798f895a6e174ef3a2183fc9578f0d22/packages/story-editor/src/components/library/panes/media/local/hotlink/useInsert.js#L111-L121

I can't reproduce any issues with audio detection.
Inserting https://c.tenor.com/Mc-DPcAOaNsAAAPo/lol-omg.mp4 -> video is marked as muted
Inserting https://sample-videos.com/video123/mp4/720/big_buck_bunny_720p_30mb.mp4 -> video is marked as having sound
With that said, I think 50720bcb9cefe1f843ca08dfa4e52f4d9b5950d7 makes this more robust.
Please test this again and provide any steps to reproduce, including video URL used.

Sorry I wasn't clear. Uploading media, has audio is broken.

Audio detection for uploads is now fixed

This needs QA instructions. This pr has changed before now, there are UX changes, like disabling the insert button and changing of text on button etc. I would also like to see this tested, with different files.

@spacedmonkey We've previously discussed moving QA to its separate ticket. There's also bug bash tomorrow which is an ideal opportunity to get even more eyes.

We've previously discussed moving QA to its separate ticket. There's also bug bash tomorrow which is an ideal opportunity to get even more eyes.

I think that was a old conversion, before UX was added to this ticket. Also, why would we bug bash a ticket that has even been QAed. That feels like a waste of time. Why not QA these changes?


Send Text With Enter Key

It would great to have an option to have the enter/send key submit the text instead of creating a new line. I know many other chat/sms apps offer this as an option in settings. Thanks for any consideration.

This is definitely a duplicate of #220 can someone mark it as such please


How to run it on Ubuntu?

Hello guys,
I'm trying to run it on my ubuntu 20.04 OS and found some problems... PyGetWindow does not have support for Linux yet;
Does someone have any suggestion to make it run on a Linux machine?
Any help about this subject I would be very grateful

Hey @kaburelabs i've been able to make a really good bot using X11/Xorg functions at Ubuntu 20.04 at Feb~May/21.
If you are able to adapt TibiaAuto12 repo using it:
import Xlib.display
from Xlib import X, protocol, XK


Docker compose fails due to missing directory /tmp/docker

Summary
The docker compose will fail as it references the directory /tmp/docker but this is not created by the Dockerfile or compose, nor is mitigated as a build setup requirement in the documentation in the docker readme or tutorial.
Workaround
Manually create the directory before building

The tmp/docker is setup so that we are able to use unix sockets thru a file share.  See: https://medium.com/@jonbaldie/how-to-connect-to-redis-with-unix-sockets-in-docker-9e94e01b7acd

Thanks for providing the background Peter.

Should be fixed by PR#119


Not properly applied styles for dark-theme

Describe the bug
clr-icon.is-green, clr-icon.is-success has less priority in dark-theme over the regular style so its brighter color is not applied in dark-theme
How to reproduce
Steps to reproduce the behavior:

Go to 'https://stackblitz.com/edit/clarity-v4-dark-theme-6rwcmx?file=src%2Fapp%2Fapp.component.html'
open dev tools and inspect the style of <clr-icon shape="success-standard" class="is-success"></clr-icon>
see that the dark-theme color(fill: #61b715;) is overridden by the regular color (fill: #2e8500;)

Expected behavior
the dark-theme color should be fill: #61b715;
Versions
App

Angular: [10.0.11]
Clarity: [4.0.0]

Device:

Type: [PC]
OS: [windows10]
Browser [chrome]
Version [87]

Additional notes
I am reporting this bug specifically for clr-icon .is-success where the dark-theme style has less priority over the regular style.
If this is a matter of a general method applying styles it might affect other components as well.

@iivanovgit thanks for reporting it. We gonna take a look at it.


Removed resource spec patches no longer required

Issue #, if available:
Description of changes:
Remove resource spec patches that are no longer necessary:

/ResourceTypes/AWS::ApiGateway::Authorizer/Properties/Type/
/ResourceTypes/AWS::SNS::Subscription/Properties/TopicArn/
/ResourceTypes/AWS::SNS::Subscription/Properties/Protocol
/ResourceTypes/AWS::CloudWatch::Alarm/Properties/Metrics

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Codecov Report

Merging #704 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #704   +/-   ##
=======================================
  Coverage   85.49%   85.49%
=======================================
  Files         118      118
  Lines        6813     6813
  Branches     1628     1628
=======================================
  Hits         5825     5825
  Misses        615      615
  Partials      373      373

Continue to review full report at Codecov.

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


k8s authentication documentation part

Thanks a lot. That's now merged, polished, and forward-ported to 2.0.x.


add _apps/template.md for app Template

For bot Template https://github.com/apps/template
Suggestion by JasonEtco from https://twitter.com/JasonEtco/status/929590076400635905

UPDATE:
Latest release Template v0.2.0
I've implemented custom template feature, with Handlebars support :D
Now you can report duplicated issue with simple command

@bkeepers 🤖  I had updated the changes for this PR.
Hopefully this PR can be merged soon.


Need Updated Documentation

vue-rangedate-picker version
2.0.x, x.y.z
Hi, Kindly update the documentation.

What part of the documentation needs to be updated?


add pfy.ch

I'm re-opening a PR for my site, its been a few months and I've been tending to it like a garden in my free time.
I've incorporated feedback from here, as well as from friends & have lowered the overall size of the site through compressing my images and implementing scroll listeners for loading high-res (480p!) images. No more 10mb jump-scares! The server is hosted in Australia so it may be a bit slow but I test all the pages on a slow 3G connection & it's much snappier than last time I requested to join the webring.
I've also populated the site over the last few months with things I've found interesting or adventures I've had. With pages on cameras I've collected as well as pages on Drains & Tunnels I've explored with a local urban exploration group. There is also programming content ranging from solutions to tough questions I've had at work as well as reference notes for future me. Lastly, since leaving twitter, I have nowhere to share my arcade scores! So I have a page with my high-scores in assorted arcade shmups :smile:
Let me know if there's any feedback! I'm always looking to improve my homepage.

I’ll update the site with an about page this afternoon 👍

I've added an about me page which is accessible from the homepage.
https://pfy.ch/about.html

Thanks for adding the about page! Welcome to the webring :)


Neolink v0.6.3-rc1

Hi,
Would it be possible to have the 0.6.3-rc1 release installed via the addon? Maybe via a UI configuration setting?
In any case, thank you for making this available

It’s not possible to make a version selector via ui cause the base image of the addon differs. What I could do is providing a second addon. So one for the current final release and the second one for the latest release. Would that help?

Hi, yes that would be great!

added neolink-latest. refresh your add-ons in HAOS, uninstall Neolink and install, configure and start Neolink-latest Add-on.

Just a heads up, using latest results in fetching the most recent repo version (in this case iterations of rc2), instead of the latest actual rc1 which is marked in the github repo as pre-release. The current version doesn't work properly due to ongoing changes, meaning it shouldn't be used.
Can you modify the docker image tag to be the latest RC?
quantumentangledandy/neolink:v0.6.3.rc.1

On the subject:
https://github.com/QuantumEntangledAndy/neolink/issues/236#issuecomment-2056998064

Rc1 was having also heavy problems, due to this fact we switched back to 0.6.2. if we now use rc1 directly instead of latest (which is rc2 today and could be rc3 tomorrow) than that would not make anything better.

Ok, understood. It's a matter of trying to use a version that seems stable enough then.


add div

👷 Deploy Preview for fancy-puffpuff-df775f processing.

Name
Link

🔨 Latest commit
c2c47df6c3d2bcaf0becaab008affa34a2dbc2fe

🔍 Latest deploy log
https://app.netlify.com/sites/fancy-puffpuff-df775f/deploys/65a6a3fda291ba00084eb23c


lib size is enormous 1.07Mb

I'm sorry if it's me doing something wrong (but i doubt it)
But bundle size is huge due to react-coverflow size.

In my opinion, it's not ok for carousel library to take 47% of bundle size

Yikes, that is entirely too massive @Stas-Buzunko, I'll take a look at ways to reduce it's size ASAP. Thanks for bringing it to my attention

Hi @Asalem1 , any updates on this one.

Alright, final update on this ticket. The published size has been reduced a bit, but it seems like the main culprit in the large dependency size is the radium dependency. My intent is to remove the dependency and see if I can replicate its functionality without losing performance. I'm going to leave this issue open until then.


Is this supported on GCC or GCC High Tenants?

Licensing seems ambiguous, not all Vars seem to have this SKU available. Additionally there is no mention of the Government tenant licensing / availability.

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

ID: a2070d20-21d1-c8bb-bbb4-697083efcd20
Version Independent ID: f4998e3c-74ed-4a2b-fe7c-c55044b9a6b4
Content: Microsoft Teams Rooms licenses
Content Source: Teams/room-systems/skype-room-systems-v2.md
Service: skype-for-business-online
GitHub Login: @LanaChin
Microsoft Alias: v-lanac

@vscurtis Thank you for submitting feedback and contributing to the docs. We are currently investigating this.

@vscurtis Thank you for your feedback. I have added GCC and GCC High Dod to the Note. This can be tracked Via PR  https://github.com/MicrosoftDocs/OfficeDocs-SkypeForBusiness/pull/2821.
Meeting SKU is available on all Versions of Enterprise and GCC licenses
Regards
Sri

GCC or GCC High and DoD SKUs are not supported for use with MTR. Hope that answers your question.

So teams meeting rooms will or will not be available in GCC / GCC High? seems we have conflicting answers here.

@vscurtis Teams Meeting Room devices don't use those particular licenses at present. This isn't the correct channel to make assertions about what licenses might be supported in the future. Should the licesninge requirements change, the page will be updated ASAP.
The OP had a question that was misinterpreted as a definitive statement or correction needed, and without checking someone initiated a PR to make the "correction," which was not approved because it was in error. If there were conflicting "answers" (speculation seems more likely) they have been overridden by the official answer from the product group.

@jambirk i understand that there is a tendency to speak in whatever lingo seems appropriate for internal groups at the moment but i am afraid you are using terms that are not clear to me as an outsider. what is 'OP' or 'PR' and how on earth does one know who is providing a definitive answer because everyone here looks to be a 'contributor'
You appear to be speaking with a tone of some authority here but assumptions are a bad thing.
Back to my original ask: our organization is evaluating the use of the MTR sku because we have some meeting room devices we are looking into to make things easier on the executives. The switch from Skype for Business to Teams was less than transparent about hardware impacts and so we avoid making the same mistakes we have been talking with experts and one of the recommendations was this license option. HOWEVER we are in the process of also migrating off of the consumer based cloud to government community cloud and we want to be absolutely certain that when we move over there any devices or licenses we get in consumer have a direct correlation in GCC and GCC High.
If we get hardware / software that utilizes the Teams Room License; will we be able to find a comparable license for GCC and if not is there some work around process that can be used instead to support that hardware?

@vscurtis
I am the original writer of this article, but for licensing questions I always check with the development team that creates the product. The article is accurate and doesn't require a correction.
OP (in this case you) refers to Original Poster, which has been parlance in threaded discussions for a long, long time. PR is a Pull Request (at least on GitHub) and msbemba is a freelancer who acted prematurely in creating one.
Questions about what is coming would require me to either speculate or pre-announce a change, and that's not appropriate in product documentation.  I simply can't answer this question in the article or in an issue thread. Your best options are to either open a support ticket, or direct your question to the discussions in a marketing blog article like this one:  https://techcommunity.microsoft.com/t5/Microsoft-Teams-Blog/An-inclusive-meeting-experience-for-everyone-in-any-room-with/ba-p/805877


History page balances are messed up

Is it because we're so close to the new year?
https://gratipay.com/~whit537/history/?year=2016

Not sure but will check into it.

This is still being weird, so it doesn't look like a year thing?
E.g.,
 is the first week of one account. So it starts at $192.

Fixing in #4439

#4439 is merged, close?

Yep


URL preview-on-hover not visible when window is maximized

I confirm (by marking "x" in the [ ] below: [x]):

[x] I have confirmed this is an issue on the newest beta build available here
[x] This issue doesn't reproduce on web browsers (such as in Chrome). If it does, issue reports go to the Mattermost Server repository.
[x] I have read contributing guidelines.

Summary
When the app window is maximized and when hovering the mouse over a hyperlinked URL, the pop-up along the bottom left-hand edge is not fully visible; only a few pixels-worth of the top edge can be seen.
When the app window is not maximized, the pop-up is fully visible as expected.
Environment

Operating System: Windows 10
Mattermost Desktop App version: 5.1.0-nightly.20220505 commit: 19a075f8
Mattermost Server version: Team Edition 5.37.6

Steps to reproduce

Maximize the app window.
View a message that contains a hyperlinked URL.
Hover the mouse over the hyperlink without clicking.

Expected behavior
The destination URL of the hyperlink should pop up and be visible along the bottom left-hand edge of the window.
Observed behavior
The pop-up is partially visible, with only a few pixels of the top edge in view.

In the screenshot above, the thin white line just above the taskbar is the URL pop-up while my mouse is hovered over the hyperlink. The window is not behind my taskbar; it is simply maximized. I even tested moving my taskbar to the top of my screen, and the issue remained. If I un-maximize my window, however, the pop-up becomes fully visible.

@jgilliam17 Are you able to reproduce this? cc @devinbinnie

@amyblais I can reproduce yes, will create a ticket.


Enhance cross-collection searching by supporting the visibility field

Currently visibility is only handled properly when searching within collections.

This just needs the facet enabled.