Type annotations inside destructuring

It seems like the following should be supported:
var [a: String, b: Number] = ["foo", 1];

function foo([a: String, b: Number]) {}
foo(["foo", 1]);

Is this a deliberate decision or was the scope of ES6 type annotations restricted until ES6 became more widespread?
Thanks!

We've held off on adding inline type annotation for destructuring since there isn't an obvious place in the grammar for object destructuring (var {a: string, b: number} = x will bind x.a to string and x.b to number). Array destructuring doesn't have this problem, so we could definitely add it.
Also I don't think TypeScript supports these kinds of annotations, so it wasn't something we urgently needed to support. You're saying the 6to5 acorn fork has support?

@gabelevi Yeah, I was rewriting some of the flow parsing in acorn-6to5 and accidentally added it in.  Only my examples with array destructuring work correctly because as you said, syntax ambiguity.

Hmm, interesting how TypeScript / Flow are going to resolve this. Ideally, we could just do it during scope analyzing so if "property value" of object pattern is linked to type, then use it as type annotation and if not, use as destructuring target.
The only downside will be for cases like:
class A {}
var {a: A} = obj;

when you really want to override locally defined class and reassign A = obj.a, but I don't think this is an often case and it can be easily workarounded on developer side.

@RReverser You can just do:
class A {}
var { a }: { a: A } = obj;

@sebmck That doesn't look good enough - variables and their types are split into different sides of operator. It works for regular object literals, but doesn't look so nice for bindings.

I've been thinking about this for awhile, and I think the : type paradigm just isn't going to fit into the grammar for object literals, patterns and types. I think if we're going to add inline type annotations then we need to switch paradigms. I've been thinking maybe we could do something like
// Works well for object types
var { number x; string y } = { x: 123, y: "hello" };
// Works well for object patterns
function foo({number a, string b}) {}

// Not so sure if object literals need this
var z = {
  ?string m: null,
  ?number n: null
};

// because soon Basil's new expression annotation syntax will land
// which will help object literals
var z2 = {
  m: (null: ?string),
  n: (null: ?number)
}

The downsides include that this paradigm doesn't match the other type annotations and that there are certain reserved keywords in the modifier position (get, set, static).

@gabelevi, maybe I am being dense here, but why is var {a: string, b: number} = x not possible? Shouldn't the context be enough to disambiguate between destructuring-with-type-annotations and object literal syntax?

Oh, nevermind. I forgot about rebinding destructuring.

Perhaps what I'm suggesting is an unheard of thing, but can typescript change the ES 6 syntax in this particular place?
function foo({ a: number, b: string as c }) {
  return a + c;
}

The ES 6 spec already uses the "as" keyword for about same purpose:
import {a as b} from 'mod';

So it should be sort of ok to use this "as" to rebind destructured arguments.

TypeScript uses as as an coercion operator. How about the following?
var { x as number, y as string } = { x: 123, y: "hello" };

function foo({a as number, b as string}) {}

var z = {
  ?m: null as string,
  ?n: null as number,
};

Alternatively, one could also use “Basil's new expression annotation syntax” everywhere:
var { (x : number), (y : string) } = { x: 123, y: "hello" };

function foo({ (a : number), (b : string)}) {}

var z = {
  m: (null : ?string),
  n: (null : ?number)
}

But I like as, because then there is no overloading of the single colon. A double colon (instead of as) would also be nice, but IIRC that is reserved by TC39 for guards.

Hmm, I actually like { (x : number) } = { x: 123 }. Seems totally unambiguous and unlikely to conflict with future ECMA-262 proposals. Any objections? /cc @gabelevi

Wondering: would this get you into parsing trouble w.r.t. method definitions? Concise property definitions in object literals (vs. in object destructuring patterns) may be problematic.

:+1: to @rauschma's idea of using "as".

:+1: to @rauschma's other idea of using "expression annotation syntax", or what I naïvely call "casting".

This would be super useful for functional React components:
const Component = ({
  propA,
  propB,
}) =>
  ....

Currently, the best way is to create a type and access the props via field access (that way only prop. is repeated a lot, but not the names of the props).
type Props = {
  propA: SomeType,
  propB: SomeType,
}
const Component = (props: Props) =>
  ....

Clearly the destructuring and typing syntaxes clash :(. This makes sense to me, but is super ugly:
const Component = ({...}: {
  propA: SomeType,
  propB: SomeType,
}) =>

(since empty {} destructuring seems to be going through the committee, and wouldn't be obvious anyway).
This is probably currently illegal, but I have no reasoning for it:
const Component = ({{
  propA: SomeType,
  propB: SomeType,
}}) =>

What I'm looking for is a solution that lets us easily go from typed params to named typed params:
f = (a: X, b: Y) => ...
f = ({{a: X, b: Y}}) => ...

What about this? I like "as" operator too - it is short, simple and evident. This example is good:
function foo({ a as number, b as string }) {}

Just bumped into this myself. I also support { (x : number) } = { x: 123 } since it aligns with type annotations elsewhere. I don't think we need to add a new keyword.
For some reason I thought { x: (localX: number) } = { x: 123 } would work, but perhaps it also should work.

This syntax is confuzy - too easy to make mistake.

I was on a long plane ride this weekend so I figured I'd toy around with the paren-grouping syntax (i.e. {(x:number)} = {x:123} because it's simple, it composes well with the rest of of our lval type annotations, and when paired with object literals it unlocks the ability to type a property slot (rather than having to type the value and inferring the property slot from that). On objects, this would unlock the ability to understand deferred initialization of a property similar to how we can with let/var (i.e. let foo:number; foo = 42; --> let a = {(someNum:number)}; a.someNum = 42;)
TBH I don't find the syntax beautiful, but I don't think we're going to find something that everyone agrees on WRT syntax, so I went with what matches the most with our other annotation positions.
My branch is linked below. It doesn't wire up enforcement of the destructuring annotations just yet -- but it  shows that we can if we wanted to pursue this route:
https://github.com/jeffmo/flow/tree/pattern_lval_annotations

Did you get in contact with the TypeScript team? Ideally, a syntactic solution would be adopted by both Flow and TypeScript.

@rauschma: I haven't on this, no. I'm happy to reach out. That said -- TS already diverged from our casting syntax with their own as, so my guess would be that they would have the same motivations for staying consistent as we would.

@jeffmo It's not about casting consistency, but rather would be nice to have as much syntax shared as possible. I'm pretty sure they were also exploring options for typing destructuring, so collaboration could help in both short-term and long-term perspectives.

Also similar syntax is good for IDE.

Indeed. I will be sure we follow up with them before moving forward with anything here

👍

I know that you probably can't switch the whole syntax over now, but it's too bad that we can't have this:
Regular function call
function (
  number id,
  string name
) {
  // function body
}

Object destructuring
function ({
  number id,
  string name
}) {
  // function body
}

Array destructuring
function ([
  number id,
  string name
]) {
  // function body
}

One of the most useful places to have types is in function signatures, so it's too bad that Flow doesn't support a cohesive and readable syntax there.

@jtremback It's not too bad at all. Flow does indeed support a cohesive and readable syntax. Destructuring aside (I'll address it below), your
function (
  number id,
  string name
) {
  // function body
}

is just (by the way a function is an arrow type, you don't have a full function type there) a personal preference over
function (
  id: number,
  name: string
): any {
  // function body
}

I prefer the latter, you prefer the former. It's not too bad.
Destructuring actually doesn't mean that your function's argument is what you wrote it is. I'm talking about the object destructuring. Your array destructuring is actually a tuple type which is [id, name]: [number, string].
The main point is — you argument is an object or a tuple. It has a specific type. And this type would be the argument type. Just because you're destructuring it the type doesn't change. So I actually prefer to declare my type elsewhere (normally you would use it in several places anyway) and then just do { id, name }: MyType. Flow will figure the rest up. The trick is to use Flow's impressive mechanisms not to try to annotate every single thing in your code.
But it's just my opinion. However the presence of one invalidates your too bad argument.

@alexeygolev What I mean is that the use of : in Flow's syntax collides with the extensive use of : in javascript. Object destructuring allows us to have named arguments in javascript, which is pretty great for readability and reduces accidental bugs. Unfortunately, the : makes this tricky syntactically. I think that @jeffmo's solution is a good compromise though.

Has there been any progress on this recently?
I also am in favour of @rauschma's suggestion:
var { (x : number), (y : string) } = { x: 123, y: "hello" };

function foo({ (a : number), (b : string)}) {}

var z = {
  m: (null : ?string),
  n: (null : ?number)
}

I'm very interested in this working as well since it would drastically simplify stateless functional component definitions in React. Currently those have to look like this:
// @flow

import React from 'react'

type Props = {
  name: string
}

const GoodMorning = ( { name }: Props ): React.Element<any> =>

    Good Morning, { name }

export default GoodMorning

Rather than this:
// @flow

import React from 'react'

const GoodMorning = ( { (name: string) } ): React.Element<any> =>

    Good Morning, { name }

export default GoodMorning

The second example is much terser and DRYer, and this is a pain point we see every day in our React codebases. (We do separate type aliases because inline within the signature gets ugly with more than 1-2 properties.)
The advantage of the paren-grouping syntax is that it's fairly intuitive since it reuses existing syntax in a way that makes sense. An inline type annotation "feels" like an expression, so "disambiguating" it with parentheses within a destructuring expression isn't too surprising.

The { (name: string) } solution looks great. Is anyone working on getting it implemented?

The last I heard from any member was above where @jeffmo said they'd try to coordinate on this with the Typescript team to make sure there isn't unnecessary divergence. If we get consensus on this I might like to get my feet wet trying to make a PR - I don't know if it would be wasted work though?

@jrajav
Realistically:
const MyComponent = ({
  ( aProp: string ),
  ( someOther: number ),
  ( zethIs: Object ),
  ( onClick: (event: Event) => void ),
  ( onSubmit: (event: Event) => void ),
}) =>
  ... {onClick} ...

I'd probably stick with the current:
const MyComponent = (props: {
  aProp: string,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>
  ... {props.onClick} ...

But that syntax could still be useful elsewhere.

@xixixao I'm confused - doesn't your second example still conflict with native object destructuring syntax?

@jrajav The second example is actually the current syntax.  The parameter is not destructured, simply annotated with a separate object expression.

@ajhyndman Ah! sorry - I missed the top line. Yes, that does work currently but is clunky when you are using actual destructuring in the signature itself (which is all over the place in a modern React app) - it results in a lot of repetition. I think it's far from ideal.
const MyComponent = ({
  aProp,
  someOther,
  zethIs,
  onClick,
  onSubmit
}: {
  aProp: string,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>

@jrajav My point was that if the syntax required parens around each field, I'd prefer the current pattern that doesn't use destructuring at all, which would kind of go against the whole point of coming up with this syntax :/.

I'm not sure how it defeats the point, if the point is to make annotation terser and reduce repetition. Can you elaborate on that?
I can see how one might think it's a bit ugly but I think it might be the best choice at hand, regardless.

I'm just spitballing... We wouldn't have any of these problems if Flow's type syntax didn't overlap with the ECMAScript syntax for destructuring aliasing.
If, for instance, Flow had borrowed the type declaration syntax from Haskell, we'd be able to do this:
1.
const MyComponent = ({
  aProp:: string: anAlias,
  someOther:: number,
  zethIs:: Object,
  onClick:: (event:: Event) => void,
  onSubmit:: (event:: Event) => void,
}) =>
  ... {onClick} ...

Or, if ECMAScript had opted for the same alias notation in destructuring as with named imports:
2.
const MyComponent = ({
  aProp: string as anAlias,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>
  ... {onClick} ...

What about leaning on the ES6 computed property names syntax?
3.
const MyComponent = ({
  [aProp: string]: anAlias,
  [someOther: number],
  [zethIs: Object],
  [onClick: (event: Event) => void],
  [onSubmit: (event: Event) => void],
}) =>
  ... {onClick} ...

const obj = {
  [aProp: string]: 'string',
  [aMethod: Function]: () => {
    ...
  },
};

The only thing I'm not certain of is whether or not that's ever going to be ambiguous with a preceding expression in objects.  I guess it could also lead you to the mistaken assumption that you can use an expression to resolve the name of the key you wish to destructure from a parameter.

[someProp: string]: 'hello' is not the same as someProp: 'hello'. It's same as [someProp]: 'hello'. Also it makes it look like you're specifying the type of the key, not the value.

Does someone work on getting those things @ajhyndman done? It is something which makes flow a bit lagging.
We are used to write this:
export default ({ isActive }) => (
  <MyAwesomeComponent isActive={isActive} />
);

So how can I set isActive type?
Or even worse, if I would set isActive alias this way: ({ isActive: active }) it is correct ES2015+ syntax. Seems like I am unable to use it with flow :(

For your example, I'd just do:
export default ({ isActive }: { isActive: boolean }) => (
  <MyAwesomeComponent isActive={isActive} />
);

Which is very reasonable for annotating a single prop. Components often have many props, however, at which point we use:
type Props = {
  isActive: boolean,
  className: string,
  label: string,
};
export default ({ isActive, className, label }: Props) => (
  <MyAwesomeComponent isActive={isActive} className={className}>
    { label }
  </MyAwesomeComponent>
);

Yes, exactly this way I use flow with react, but what about function aliases, how should I use them with flow?
e.g.:
const doLog = ({ veryLongPropertyWhichICannotChange: prop }) => {
  console.log(prop);
}
doLog({ veryLongPropertyWhichICannotChange: 1 }) // 1

type P = { veryLongPropertyWhichICannotChange: number };
const doLog = ({ veryLongPropertyWhichICannotChange: prop }: P) => {
  console.log(prop);
}
doLog({ veryLongPropertyWhichICannotChange: 1 }) // 1

Above code sample in flow repl

@acusti awesome! Thanks!

I acknowledge that I'm very late to this party, but is there any value in leveraging the assumption that a Flow type-comment will always be a type assertion?
For instance, I feel like this is pretty unambiguous:
function foo({
  x/*: string */: inputXValue,
  y/*: number */: inputYValue,
}) {
  // ...
}

I haven't read the code that reads in the type-comments, so if the type assertion in the comment is transparently added to the AST at read time, then it might require too much work to implement. The syntax isn't perfect, especially not if you're used to using Flow types as a first-class part of the language, but I figure it's always better to voice an idea.

I just hit this today in my first 5 minutes with Flow -- I went over-eager on adding : type and put it in an object destructuring in a function call too. Although the first use will get an error, why doesn't destructuring (or any assignment) to a type name get a Flow warning immediately?
(This newbie likes jeffmo's proposal too.)

What about this:
const { (a: AType): aa, (b: BType): ba } = c;

@akomm It does seem like that syntax has the most support.  I believe it has been suggested above a few times already.  👍
https://github.com/facebook/flow/issues/235#issuecomment-174792066

lol I must have missed it! :D

Just to add my 5 cents...
For me this issue is pretty important, because destructuring is the only way to simulate passing named arguments in javascript.  Declaring type information separately from the function definition doesn't help too much because it doesn't eliminate verbosity and repetition. So for now the only option I see is to use object or some other weak type, and it makes me unhappy :(
It would be awesome if we could somehow find a solution to this issue in the foreseeable future.

+1

New to Flow and testing it out a bit for a project.  Really liking it so far 👍!  I think I'm running into something that might be related to things discussed in this issue, but not sure yet.
I don't really feel the need to add typings to destructured object-argument variables, but I'm one of those who likes to separate types from my code where possible... personal preference :).  Using functional type signatures, I can get some nice usage out of Flow so that component instances are created with expected props wherever they are used.
However, I noticed a difference with destructured variable type checking when using a functional type signature vs inlined arg and return types (v0.48.0).  Maybe this has already been brought up in a separate issue (couldn't find one quickly) but I'm seeing a difference in behavior with these approaches, and wondering if these two examples should be treated as semantically identical, or if maybe I'm just misunderstanding something :)
// @flow
import React from 'react'
import type { Element } from 'react'

// generic type signature for functional React component
type ComponentFn
 = (props: P) => Element<*>

// define props
type Props = {
  foo: string,
  bar?: boolean,
}

// using function type
const MyComponent1: ComponentFn<Props> = ({
  foo,
  bar = 'false',
}) => (

{bar &&
{foo}
}

)

// No Error when using invalid default prop `bar`
const test1 = () => <MyComponent1 foo="test" />

// using argument & return value types
const MyComponent2 = ({
  foo,
  bar = 'false',
}: Props): Element<*> => (

{bar &&
{foo}
}

)

// Error when using invalid default prop `bar`
const test2 = () => <MyComponent2 foo="test" />

above snippet in flow repl

How about this?
function foo({surname string, birthDate Date})

If a code..
function foo(dto: {surname: string, birthDate: Date})

.. need to be improved to use destructured object, instead of this proposed syntax..
function foo({(surname: string), (birthDate: Date)})

.. which entails going to each variable and surrounding each of them with parenthesis, it's tiresome and makes the code looks so busy. It would be easier to just remove the colon:
function foo({surname string, birthDate Date})

And when it's need to be renamed, just use the already established syntax for renaming:
function foo({surname string: apellido, birthDate Date: fecha})

Maybe ES6 committee decided that renaming destructured object would be the norm, so they choose a terse syntax to rename something. But why ES6 didn't dream of JavaScript of having  types someday, or having TypeScript being part of the browser? Had they thought of it, they would not choose colon for renaming variable, they would use as for renaming, which is symmetrical with import statement. Another alternative would be is not to use an operator at all, same with SQL:
SELECT * FROM Person as p

Not using as would be just as fine:
SELECT * FROM Person p

So renaming surname to apellido would be:
function foo({surname apellido, birthDate fecha})

const {surname apellido, birthDate fecha} = person;

And then when in the not-so-distant future that JavaScript would have types. Boom! they could just use the established syntax for declaring types:
function foo({surname apellido: string, birthDate fecha: Date})

const {surname apellido: string, birthDate fecha: Date} = person;

function foo({surname: string, birthDate: Date})

const {surname: string, birthDate: Date} = person;

Too late.
Will just wait for WebAssembly to take off, and wait for a language that has a nicer syntax :) It might also be our chance for a language to have a decimal type.
I highly suggest this syntax..
function foo({surname string, birthDate Date})

..it's not really weird to remove an operator for declaring types, e.g.
CREATE TABLE Person (
    PersonID  int,
    LastName  varchar(255),
    FirstName varchar(255),
    Address   varchar(255),
    City      varchar(255)
);

Another advantage of the above over this one..
function foo({surname as string, birthDate as Date})

..is if ECMAScript committee decided to make the renaming use the as keyword so there's symmetry with import's as, this won't cause breaking change:
function foo({surname string as apellido, birthDate Date as fecha})

function foo({surname as apellido, birthDate as fecha})

const {surname string as apellido, birthDate Date as fecha} = person;

const {surname as apellido, birthDate as fecha} = person;

Whereas this one would cause breaking change when as would be introduced for renaming destructured object. Both surname and birthDate would have any type when as would be used for renaming in the future:
function foo({surname as string, birthDate as Date})

const {surname as string, birthDate as Date} = person;

Nearing a year out and we still haven't heard anything about the mentioned coordination with Typescript @jeffmo - since it seems the parentheses syntax last suggested by @akomm but by others too has a plurality of consensus. It seems unlikely we'll be able to find a syntax that meshes perfectly with native syntax, looks nice, and won't conflict with anything else - the parentheses version is probably the closest we'll get. Can we call it good enough and get going on implementing it?

Why should this be any different from the rest of Flow? To me, given how flow works with ':', the following is natural (and what I tried on a guess):
const FunctionalComponent = ({prop1, prop2, ...rest} : {boolean, string, Object}) => (

    {doSomethingWith(prop1, prop2)}
    <ChildComp {...rest} />

)

This seems the cleanest and most likely lest hacky to implement option, IMNSHO.

@Panoplos This would clash with the existing (working) annotation syntax:
const FunctionalComponent = ({ prop1, prop2, ...rest }: { prop1: boolean, prop2: string }) => (

    {doSomethingWith(prop1, prop2)}
    <ChildComp {...rest} />

)

Why are the second prop names necessary?

Because it's inline type definition of object that destructuring
It's similar as:
type PropsType = {
  prop1: boolean,
  prop2: string,
}

const FunctionalComponent = ({ prop1, prop2, ...rest }: PropsType) => (

    {doSomethingWith(prop1, prop2)}
    <ChildComp {...rest} />

)

I can live with this version. Thanks for the pointer! ;^)

@wKich That work around doesn't always work:
//@flow

type input = {
  a?: number,
  b?: string,
  c?: null|number // this causese an error
};

function takeOptions ({ a = 1, b = 'abc', c = 1 }: input = {}) {
  console.log(a);
  console.log(b);
}

takeOptions({ a: 2 });
takeOptions();
takeOptions({});

// this should be an error
//takeOptions({ a: 'string' });

https://flow.org/try/#0PTACDMBsHsHcCh4BcCeAHApgAgJYDs0BXJLAXiwG94ssBDAfgC4s9CBbAIwwCcAaarByZYAzkm74A5vxoBjYa0iQAPq048sILEgAWOEVlm1CIjKbp4sPbtG7wAvgG5E4QnllIc0S0loBrDAB5NE9vAwAKCjoyLABGXkEYgHJaDlkkhNkY2Kx7ZnwiEnIKewBKSgFZMOhIDAA6GElw2lLnOWrahugmjlaHRF8A4NC8EUi6ZgAmXL7BoJCvUfDZ-3mRsZK++C1dfVEdaEJIABNBbFpLa1tt4DnhxY2JrCSxCTxJJJnHIA

Alright, I can see a little stagnation here (or some issue with decision making), so I'd like to sum it up (again?) and move it forward.
Some proposals of syntax evolved during this two and a half year long (!) discussion. All of them vary in details only, some extra braces or colons appear here and there, but general concept stays the same, as far as I see.
In some cases there was no example presented for property alias nor for default value, so I extrapolated the idea by myself. I hope original inventors will forgive me this boldness.
Proposal A
function f({ a as string, b as number }) {}
// extrapolating idea to aliases and default values
function f({ a as string: aAlias, b as number: bAlias }) {}
function f({ a as string: aAlias = 'a', b as number: bAlias = 1 }) {}

Proposal B
function f({ (a: string), (b: number)}) {}
function f({ (a: string): aAlias, (b: number): bAlias}) {}
// extrapolating...
function f({ (a: string): aAlias = 'a', (b: number): bAlias = 1 }) {}

Proposal C
function f({ a string, b number }) { }
function f({ a string: aAlias, b number: bAlias }) { }
// extrapolating...
function f({ a string: aAlias = 'a', b number: bAlias = 1 }) { }

Proposal D
function f({ a:: string, b:: number })
function f({ a:: string: aAlias, b:: number: bAlias })
// extrapolating ...
function f({ a:: string: aAlias = 'a', b:: number: bAlias = 1 })

As far as I see nobody found serious issues with any of these proposals, it seems to be a matter of opinion and consistency with current syntax. Maybe it's a bias, but it seems idea B got the most support from the community so far.
@mroch @gabelevi @samwgoldman, what do you think about this?

Proposal B fits best into current JS style

C is cleanest...

@Panoplos but inconsistent with regular variable typing (a: string) vs ({a string})

I dunno... This B seems cluttery to me. I would just as rather stick with the existing mechanism than adopt something like B. I am just one fish in this vast ocean, though...

Proposal B is consistent with existing syntax around declaring variable types, casting, etc. I like it and I think it's the most intuitive. It can be verbose in functions with lots of params but that's avoidable by wrapping params on new lines or just falling back to the status quo of using a named object.

@Panoplos whatever is chosen at the end of the day, it'll be optional I suppose. If you don't like any of proposed ideas nor have another one, it's okay and will be okay to keep using current syntax.

Proposal A is easiest to read for me.

a as string: aAlias from A is confusing. ES6 module imports use as to alias, so this suggests to me that a is being aliased to string which has a type of aAlias. With basic types, this less of a concern, but what about when you have { factory as WidgetFactory: widgetFactory }?
B seems the most intuitive. It looks like an existing cast, doesn't introduce new keywords or a new syntactic relationship between variables and types, and is easily explained: if you need to alias a typed property of an object, wrap property: type in parens.

@jordansexton You're right, I didn't understand original idea I suppose. Added two more variants of it.

@erykpiast Nice addition. And I'm not sure I understand the original idea either 😄

A.3 looks the best. {a: string as alias = 'default}

Yes, A.3 is the best, now. Very intuitive.

A.3 (or A.2) is clashing with current syntax, so that's probably a no-go.
A.1 and D use a different token for :. C uses only position. All widely inconsistent.
B is consistent, but onerous. As multiple people mentioned, if that was the syntax, they might prefer not to use it all:
const Quote = ({
  (message: string),
  (quote: quote),
  (name: string),
  (photoUrl: string),
  (photoAlt: string),
}): ReactElement => (

    {message}

      <q>{quote}</q>

— {name}

)

Going from f(a: A, b: B) to f({a, b}) (f({(a: A), (b: B)})) is not straightforward.
But no one came up with other intuitive syntax, so it's probably either add B or leave as is.

somebody should make the decision now I think.

Yes, it's time for this.

Useless bump vote comment for moving forward with B...

@mroch @gabelevi @samwgoldman, I'm just trying to get your attention :)

how about an inline-comment based solution? I remember having a conflict between eslint and flow and using a /* */ comment made both happy

I prefer propose C:tada:
ref: https://github.com/facebook/flow/issues/235#issuecomment-343083579

FWIW This is my biggest obstacle to using Flow.  Destructuring is too powerful and expressive to sacrifice or cripple for static typing. Proposal B please.

I also vote for variant B.

Variant B. It's consistent with current syntax.
And do you have some ETA? The issue was created more then three years ago...

please consider adding b. i prefer it to the choice that typescript went with, which is to write the destructured structure twice, once for the matches, and the other for the type names

Happy belated birthday #235! Sorry we missed it... 3 years is a great age, you should stop drooling soon. cc @lili21

B is noisy and error - prone.
D have no benefits over A and C.
So my vote is either A or C. Either of them is ok and much better than current situation.

@dmitry-lexical in what way is B error-prone?

At this point I'd take any proposal as long as it makes this move forward...

It would be cool if the Flow team could provide us with a roadmap of what they are working on, and to see the priority of issues like this.
It feels so many issues are being left behind.

Hi, proposal B is the most likely one the Flow team would support, but it continues to be very low on our priority list. Happy to take PRs.

Hey Avik, thanks for taking the time to reply! I was wondering if the Flow team would consider using GitHub to discuss progress/plans/etc for Flow. It seems like a lot of it is happening through internal groups at Facebook, and I think it would be great for the Flow community if they could be a part of it.

@gabelevi says:

We've held off on adding inline type annotation for destructuring since there isn't an obvious place in the grammar for object destructuring (var {a: string, b: number} = x will bind x.a to string and x.b to number). Array destructuring doesn't have this problem...

@samwgoldman says:

@gabelevi, maybe I am being dense here, but why is var {a: string, b: number} = x not possible? Shouldn't the context be enough to disambiguate between destructuring-with-type-annotations and object literal syntax?
Oh, nevermind. I forgot about rebinding destructuring.

I had the same question, and I still don't get it. What's the problem with var {a: string, b: number} = x ?
Why would it be bad to bind x.a to string and x.b to number?

@ESRogs var {a: string, b: number} = x is already valid javascript equivalent to var string = x.a; var number = x.b;. (It's a type of destructuring assignment. The logic of its design is easier to get with silly symmetrical examples like var {p: foo, q: bar} = {p: 42, q: true};.)

It should be noted that proposal A may be confusing if the as destructuring patterns Stage 0 ECMAScript proposal is accepted.

@Extarys, var f = function({x}: {x: T}) { /* ... */ } is working for me.
I can't really find documentation for it, although https://flow.org/en/docs/types/objects/#toc-exact-object-types and https://flow.org/en/docs/lang/refinements/ imply it.

As somebody late to the world of flow and this party of a thread... what's the likelihood that something along the lines of proposal B from @erykpiast's comment back in Nov 2017 will be integrated in to flow? That makes so much sense and instills DRY principles - the current implementation certainly has me wanting to just use {} or Object when faced with somewhat involved object destructuring.

@thisissami (and others finding this thread, as it's gotten pretty long): the status was summarized by @avikchaudhuri in a comment about 6 months ago:

Hi, proposal B is the most likely one the Flow team would support, but it continues to be very low on our priority list. Happy to take PRs.

So, the likelihood of it happening is up to you! And me, and "us" -- whoever wants to have some fun with the Flow implementation to work up a PR.

It's been nearly three years since the solution was first mentioned and it still doesn't work. How is this not facebook's top priority with flow? I'd been using it for less than a day when I ran into this.

Facebook’s priorities changed in November 2016, as you might remember.
Sent with GitHawk

What does it mean? Is flow no longer meant to be used for js type
declarations?
Śr., 10 paź 2018, 16:02 użytkownik Ermolay Romanov<EMAIL_ADDRESS>napisał:

Facebook’s priorities changed in November 2016, as you might remember.
Sent with GitHawk http://githawk.com
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/facebook/flow/issues/235#issuecomment-428583659, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAtQF7UfIbP3tlOtJpP-jZMRA5mRyUPcks5ujf3wgaJpZM4DXqCr
.

What does it mean? Is flow no longer meant to be used for js type declarations?

That was a joke, it's implying that political issues are distracting Facebook from tech as a priority. It's not actually clear if the events of the last few years have made flow less important.

How does this look? It incorporates current syntax like B, but it doesn't require pairs of parens. Also, the parser can't confuse it with destructuring.
function f({ a: string:, b: number: })
function f({ a: string: aAlias, b: number: bAlias })
// extrapolating ...
function f({ a: string: aAlias = 'a', b: number: bAlias = 1 })

How does this look? It incorporates current syntax like B, but it doesn't require pairs of parens. Also, the parser can't confuse it with destructuring.
function f({ a: string:, b: number: })

@chaimleib Parser CAN be confused, like here:

function f({ a: MyClass; })

Is this destructing or typing?

No, in my syntax, that would be function f({ a: MyClass: }), with a colon, not a semicolon. I've never seen semicolons inside of curly braces like that before, what does it do?

Try Flow gives parse errors on both
function f({ a: MyClass: }) {}

and
function f({ a: MyClass; }) {}

Both syntaxes seem to be available. But between these, I still prefer the one with colons.

@chaimleib Types for function parameter destructuring already work just fine. This thread is about variable assignment, where there is no solution.

So... what exactly is the issue now? And I'm asking for "I don't like that particular syntax" answers, it's a given that a number of people won't like whatever is chosen.

@lll000111 Currently, we have to do things like this to get object destructuring with type annotations:
const MyComponent = ({
  aProp,
  someOther,
  zethIs,
  onClick,
  onSubmit
}: {
  aProp: string,
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void,
  onSubmit: (event: Event) => void,
}) =>

If we need to rename, add or remove a parameter, we have to modify two different lines to make sure the type annotations and the destructured fields stay in sync. And this line-doubling gets really annoying when you are destructuring lots of fields at once.
We can't combine the destructuring with the annotations to get the effect we want:
const MyComponent = ({
  aProp: string, // actually, would rename aProp to string
  someOther: number,
  zethIs: Object,
  onClick: (event: Event) => void, // invalid syntax
  onSubmit: (event: Event) => void,
}) =>

So we are discussing alternate syntaxes that would let us stop repeating ourselves. My syntax proposal follows a nice summary post of other syntaxes above.

The UX is pretty bad, it's like either flow or ES6..ES2019 😞
Please think of something, o enlightened ones! 🙏

Actually we can't even rename a variable during destructuring, which is not ok at all:
const { SecretString: dbSecret }: { dbSecret: string } = await secretsManager.getSecretValue()

Gives the following error:
property `SecretString` is missing in  object type [1].

@SherloxFR You can rename a variable during destructuring. That’s the primary reason why this issue hasn’t yet been addressed: to avoid conflicting with that syntax. In your example, you need to annotate the original object that you get from secretsManager.getSecretValue(), not the destructured and renamed variable:
const { SecretString: dbSecret }: { SecretString: string } = await secretsManager.getSecretValue()

why not utilize cast-through any here?
    for (const [,{name, value}]/*:[string,{name:string,value:mixed}]*/ of (Object.entries(data)/*:any*/)) {
        const _name = name.replace(/\[]$/, '');

see also: https://stackoverflow.com/a/45068255

We banned this completely last year, since it's confusing.


Use Satellizer to get token and user data

Hi, I'm building an app where I need to retrieve user data from various accounts such as facebook accounts etc. I also need to get and store the tokens in a database.
Can I use this module to do this. So far, I haven't found a way to do this.
Thanks

Try reading the example which has all the code for that.

I'd appreciate it if you could point me to the said example. Thanks

https://github.com/sahat/satellizer if you look at the listing of the files - it has a folder called examples, from there you should be able to find what you need...

Yes, I am surprised you had to ask because the included examples do pretty much just that.


[CRITICAL] search, atom-scan fail to search item when file was modified

And getDirectoryForProjectPath is NOT available on atom v1.6.
CC @brumm

v0.44.2 is out!


Use pwm esc unconditionally

Drop pwm_out_sim
Enable pwm_esc for HITL mode
Allow pwm_esc to be used at the same with UAVCAN ESC

Since actuator_output max, min and disarmed default values have been changed for HITL (similar to real drone), the flight behavior of supported HITL models might be different, previous default HITL actuator_output max, min and disarmed values have been added to HITL configs in ssrc_config to preserve expected model behavior for now. Complete adaption (asimilation HITL model to real drone) will be done latter in a separate PR.


Poedit extractor doesn't fetch WordPress page template name

<?php
/**
 * Template Name: Name Template
 *
 * The template for the name page.
 *
 * @package Theme
 * @since Theme 1.0
 */

This is extracted by GlotPress and Grunt.

`<?php
/**

Template Name: Name Template

The template for the name page.

@package Theme
@since Theme 1.0
*/

`
This is extracted by GlotPress and Grunt.

Poedit has support for extraction from plugins and themes, not templates, and "template" doesn't seem to be a translatable WordPress entity as far as I can tell, so I'm not sure just what exactly is it that you're asking for here.
If you think that's something that should be supported, please provide explanation of how that should work, how it relates to translation, themes or plugins. Some pointers to relevant WordPress documentation would be nice too...
P.S. Please attach files instead of pasting mangled, corrupted copies as comments. Or at least paste into code blocks, as I edited your comment.

In short: what am I missing here?

Hi Vlasick, it's supported by translate.wordpress.org
Download https://wordpress.org/themes/hestia/ for example and use your extractor and compare it to this one.
https://translate.wordpress.org/projects/wp-themes/hestia
3 strings missings

Hi Vlasick

I've been called names in the many years past, by people having trouble with Slavic names, but this particular mangling is something truly special 😂 Thanks for making my day.
Thanks for the pointer, I see what you mean now — apparently this part of a theme can be visible in (potentially localized) admin UI. Exhaustive details at https://developer.wordpress.org/themes/template-files-section/page-template-files/
Will be fixed in 2.0.3.

Happy if I made your day "Vslasick". hahaha
Thanks for the fix.

Hi Vslavik,
how does this actually work? When I'm creating .POT file from my theme I don't see the template names in there?
I've got a template file which starts with:
``

@alleknalle This is a Pro feature.


how to config subflow

in Previous version the parent workflow relate to the child workflow by field "id",Workflow does not has filed "id",how the parent workflow relate to the child workflow

The change was to make the subflow path relative. The following is an example. If you use a value without a starting /, the engine looks for a subflow within the directory of the flow getting executed. You can use a starting / for absolute paths but .. notation as well. If it is not starting with / it is relative. The docs in getting started need to be updated and we are working on it.
description: A simple 'action' state that sends a get request
functions:
- id: sub
  workflow: ../mysubflow
  type: subflow
states:
- id: getter
  type: action
  action:
    function: sub

Cool, open a new issue if there are any questions.


Incorrectness in surrogate loss

https://github.com/reinforceio/tensorforce/blob/172022d6a8e98c34b971dd72f208799c6f583859/tensorforce/models/ppo_model.py#L110
In original paper (https://openai-public.s3-us-west-2.amazonaws.com/blog/2017-07/ppo/ppo-arxiv.pdf), equality 7, take min from clipped ratio and ratio.

You're right, I just fixed this.


Removing duplicate entries in participant selector

Critical Changes
Changes
Filtering out the availableRows array before adding the deselected value to avoid duplicate values
Issues Closed
New Metadata
Deleted Metadata
Definition of Done
Refer to Asteroids DoD document to see any additional details for the items below
- [ ] Any net new LWC work has Sa11y tests & 50% or above lines JEST test coverage
- [ ] CRUD/FLS is enforced in Apex
- [ ] Permission sets are updated to account for CRUD|FLS|Tab|Classes
- [ ] Field sets are updated to account for new fields
- [ ] UX approval or UX not necessary

[ ] Link the pull request and work item by PR comment and Chatter post respectively, e.g. GUS: W-0000000: Work Name (https://github.com/SalesforceFoundation/PMM/pull/303)
[ ] All acceptance criteria have been met

[x] Developer
[ ] Code Reviewer
[ ] QA

[ ] PR contains draft release notes
[ ] QE story level testing completed

W-9933291


Add optional backplane escalation for logextractor.sh

This PR adds an optional --backplane argument to the logextractor.sh helper script. This option is intended to assist people in using the script in Hive environments where actions such as retrieving the S3 bucket secret are normally restricted.

/lgtm


Automatically generate canary release

The canary release has not been updated in 2 weeks. It'd be great if the canary release was built off main each time a new PR was pushed. That way we can test our samples using the latest build.

Yeah, we should definitely add CI support, at least for building Fermyon.Spin.SDK nupkg.
The other artifacts are currently built by hand, but we'll be able to switch to official builds once https://github.com/dotnet/runtimelab/pull/2614 is merged and Mac and Linux are supported in the runtimelab CI (e.g. https://github.com/dotnet/runtimelab/pull/2605), so any CI efforts for those artifacts should be directed to that repo.


[Bug]: Can't change Game's details in Firefox

Give a brief about the bug ✍
In Firefox, even if I change the game's number of rows, number of players, or number of columns, there is no change in the game. It is working as expected in Google Chrome.
What is the expected behavior? 🤔
Description
Provide step by step information reproduce the bug 📄

Open the index.html in Firefox
Change the values in the input field
Press Start
Regardless of the value/s entered, the game will look like this:

Select program in which you are contributing
GSSoC24
Code of Conduct

[X] I follow CONTRIBUTING GUIDELINE of this project.

@ayush-t02 please check this out


Port over AppVeyor support to master

Fixes #210.

Looks good to me

Could you please describe the difference between master and fsharp4 in the readme. I'm never 100%  sure to which branch I want to contribute. Thx

LGTM


Us09 poste

Coverage increased (+1.2%) to 13.843% when pulling 95482fedcdefd8ad3d7f138ba7ede59865c93046 on Us09-poste into 7143d05956afeea9cf9b6acd040c9404abb3faae on master.


API stability in releases

I help manage the libsvm conda package on the conda-forge channel. Does libsvm follow semantic versioning (https://en.wikipedia.org/wiki/Software_versioning#Semantic_versioning, https://semver.org) where API changes are only in the "major" version number (i.e. "3") and not in the "minor" version number (i.e. "35")? If so, we can add some additional information for conda to use when trying to create compatible environments. A PR to add that information is at https://github.com/conda-forge/libsvm-feedstock/pull/36
Thanks!

Yeap, you're basically right.


Could not open resolvers

I am getting error:
Running : Nuclei Medium
[FTL] Could not open resolvers file: open /root/Tools/resolvers_trusted.txt: no such file or directory
but the file exist in that directory

Fixed https://github.com/six2dez/reconftw/commit/cc5dbc606ae40ae518e895cc66a91bc77126572d

This issue has been fixed.
Please run install.sh updater script again
./install.sh


Reject Django for database operations

Django is bad at handling connections to database. In particular, it doesn't close them properly.
It is known of leaking database connections - see https://github.com/TheRacetrack/racetrack/issues/319
On one of our environments, 3 instances of Lifecycle used up to 200 connections and wanted even more. It caused exceeding max pool size which caused database connectivity to misbehave. According to the graph, at some point, one instance established 94 TCP connections.

We have to put an end to this.
This crazy amount of connections comes from the fact that every thread uses its own database connection (at least one + extra leaks). One replica of Lifecycle runs on up to 60 HTTP worker threads.
Let's switch to something else than Django for database connectivity. Perhaps doing raw SQL queries with shared psycopg conenction pool would be the best choice to make it simple and to control the number of connections in a reliable, predictable manner.
At this point Django is used for the following things:

Reading & writing models to database tables - making SQL queries + Object-relational mapping
Admin panel for managing models in the database - adding ESCs, adding permissions, etc.
User management - passwords encryption, enforcing password policies, etc.
running migrations after changing SQL schema of tables.

So the problem was Django, and not Postgres being 'slow'? Or was it an unfortunate combination of the two?
Reading the above, I am in favor of removing Django database connectivity, but how many of the above functions from Django can we keep? I would prefer not to get into the habit of writing SQL queries,

So the problem was Django, and not Postgres being 'slow'? Or was it an unfortunate combination of the two?

In a sense, it was both. On prod server, it was mostly due to other apps makind Postgres slow. On test server it was Django  killing the database limits.

I am in favor of removing Django database connectivity, but how many of the above functions from Django can we keep? I would prefer not to get into the habit of writing SQL queries,

Maybe we can keep Django for other stuff, but at least use something else for crucial queries. I wonder if that can cause even more connections though.
It's hard to find the drop-in replacement for all of the reasons we're using Django.


thehive app enhancement

Thehive App enhancement.

Add query search function.
Replace String with ContainsString. String is deprecated and won't be ported to TheHive 4.

ref: https://github.com/TheHive-Project/TheHive4py/blob/1.x/thehive4py/query.py

Thanks!! This looks like it should help find other similar cases more easily for sure.
Do you mind sharing how you're using it right now?

Thanks!! This looks like it should help find other similar cases more easily for sure.
Do you mind sharing how you're using it right now?


feat: knex instrumentation

Which problem is this PR solving?
This adds auto-instrumentation for knex.
Short description of the changes
It emits spans from SQL queries regardless of the driver. Knex makes its query builder a custom thenable instance so await knex(table)... executes the query. Elegant API, but it is a lost cause for context propagation. To battle this I'm storing context before possible awaits and use that as a parent in the query method.

Thanks, @blumamir, I think I addressed everything!

Will wait for @blumamir approval before merging this since he was the most involved approver

Will wait for @blumamir approval before merging this since he was the most involved approver

Added few more minor comments, but overall LGTM.
I don't want to hold the PR, but was reviewing it by just browsing the code. Didn't actually run it in a real environment.
If it's not urgent, I can find time tomorrow to play with it on my setup.
But that's completely optional - can be skiped it if you want to merge it already for the v0.20.0 release.

Will wait for @blumamir approval before merging this since he was the most involved approver

Added few more minor comments, but overall LGTM.
I don't want to hold the PR, but was reviewing it by just browsing the code. Didn't actually run it in a real environment.
If it's not urgent, I can find time tomorrow to play with it on my setup.
But that's completely optional - can be skiped it if you want to merge it already for the v0.20.0 release.

oops, see now that contrib v0.20.0 was already released yesterday. thought it was still in proposal.


python310Packages.pulsectl: 21.10.5 -> 22.1.2

Automatic update generated by nixpkgs-update tools. This update was made based on information from passthru.updateScript.
meta.description for python310Packages.pulsectl is: "Python high-level interface and ctypes-based bindings for PulseAudio (libpulse)"
meta.homepage for python310Packages.pulsectl is: "https://pypi.python.org/pypi/pulsectl/"
meta.changelog for python310Packages.pulsectl is: ""
Updates performed

Ran passthru.UpdateScript

To inspect upstream changes
Impact

Checks done (click to expand)

built on NixOS

Warning: a test defined in passthru.tests did not pass

0 of 0 passed binary check by having a zero exit code.

0 of 0 passed binary check by having the new version present in output.

found 22.1.2 with grep in /nix/store/qh10p3mc2hf3dg3m1q4pbz0vfh8hcv6j-python3.10-pulsectl-22.1.2

found 22.1.2 in filename of file in /nix/store/qh10p3mc2hf3dg3m1q4pbz0vfh8hcv6j-python3.10-pulsectl-22.1.2

Rebuild report (if merged into master) (click to expand)

10 total rebuild path(s)

6 package rebuild(s)

6 x86_64-linux rebuild(s)
4 x86_64-darwin rebuild(s)

First fifty rebuilds by attrpath
caffeine-ng
patray
python310Packages.pulsectl
python39Packages.pulsectl
recapp
tauon

Instructions to test this update (click to expand)

Either download from Cachix:
nix-store -r /nix/store/qh10p3mc2hf3dg3m1q4pbz0vfh8hcv6j-python3.10-pulsectl-22.1.2 \
  --option binary-caches 'https://cache.nixos.org/ https://nix-community.cachix.org/' \
  --option trusted-public-keys '
  nix-community.cachix.org-1:mB9FSh9qf2dCimDSUo8Zy7bkq5CX+/rkCWyvRCYg3Fs=
  cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=
  '

(The Cachix cache is only trusted for this store-path realization.)
For the Cachix download to work, your user must be in the trusted-users list or you can use sudo since root is effectively trusted.
Or, build yourself:
nix-build -A python310Packages.pulsectl https://github.com/r-ryantm/nixpkgs/archive/89034aaf0e01b08fcf0eca51d4671624ecfa626d.tar.gz

After you've downloaded or built it, look at the files and if there are any, run the binaries:
ls -la /nix/store/qh10p3mc2hf3dg3m1q4pbz0vfh8hcv6j-python3.10-pulsectl-22.1.2
ls -la /nix/store/qh10p3mc2hf3dg3m1q4pbz0vfh8hcv6j-python3.10-pulsectl-22.1.2/bin

Pre-merge build results
We have automatically built all packages that will get rebuilt due to
this change.
This gives evidence on whether the upgrade will break dependent packages.
Note sometimes packages show up as failed to build independent of the
change, simply because they are already broken on the target branch.
Result of nixpkgs-review run on x86_64-linux 1

6 packages built:

caffeine-ng
patray
python310Packages.pulsectl
python39Packages.pulsectl
recapp
tauon

Maintainer pings
cc @mweinelt for testing.

#154969


Added multiple language support system

i want to implement a multiple language support systems to the home page for scall your website globally for all over the world

No need for this feature...it's already present...no need to add this feature for index.html


chore: Improves the native filters UI/UX - iteration 7

SUMMARY
Improves the native filters UI/UX - iteration 7

Changes the column select to use the Ant Design one
Removes horizontal scroll when the column select is too wide
Creates the first tests for the FormConfigModal component
Fixes https://github.com/apache/superset/issues/15000
Makes the fixed tabs take up the whole row in the modal
Fixes the pre-filter to consider the ad-hoc or time range when validating

This work is part of the Native dashboard filter project
The items below will be handled in the next iterations:

Unify the select components to use the AntD one. Currently, we have different selects with different themes and interactions.
Comments from @rusackas

The iterations below are optional but recommended:
Split the FiltersConfigForm into smaller components to make it easier to read and remove react warnings
@villebro @rusackas @junlincc
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
https://user-images.githubusercontent.com/70410625/121020614-e8c19080-c776-11eb-8f64-fac5de098f09.mov
https://user-images.githubusercontent.com/70410625/121022105-6cc84800-c778-11eb-8c1d-2e5e9b498ec0.mov
TESTING INSTRUCTIONS
Check the before and after videos to see all the problems and fixes.
ADDITIONAL INFORMATION

[x] Has associated issue:
[x] Changes UI
[ ] Includes DB Migration (follow approval process in SIP-59)

[ ] Migration is atomic, supports rollback & is backwards-compatible
[ ] Confirm DB migration upgrade and downgrade tested
[ ] Runtime estimates and downtime expectations provided

[ ] Introduces new feature or API
[ ] Removes existing feature or API

Thank you so much @michael-s-molina for the PR,
small comment, let let the fixed tabs take up the whole row in the modal

Thank you so much @michael-s-molina for the PR,
small comment, let let the fixed tabs take up the whole row in the modal

@junlincc has eagle eyes!

@junlincc @villebro
Latest changes added to the PR's description:

Makes the fixed tabs take up the whole row in the modal
Fixes the pre-filter to consider the ad-hoc or time range when validating

/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true

I found a few things that might be worth consideration, but I don't think anything should be blocking really, so I'll approve. I'll leave this open for a bit, as I'd love your thoughts on the comments.

@rusackas Thanks for your review. All your points added much value. I'll tackle them all in the next iteration.

Thank you so much @michael-s-molina for the PR,
small comment, let let the fixed tabs take up the whole row in the modal

@junlincc has eagle eyes!

I agree! I can hardly find a carton of milk in the fridge, let alone spot stuff like this 😆


feat: Integrate report fields with backend

Details

This PR Integrates the policy report fields with the backend.
Fixed Issues

$ https://github.com/Expensify/App/issues/34198
PROPOSAL: N/A
Tests

Open any expense report.
Execute the following in console, replacing the policyID with the policyID of the policy the opened report is attached to:

Onyx.set('policyReportFields_47FB5109E0654E15', {
    text_title: {
        "defaultValue": "{report:type} #{report:reportID}",
        "fieldID": "text_title",
        "name": "Title",
        "orderWeight": 0,
        "type": "formula",

    },
    field_id_TEST: {
        "defaultValue": null,
        "fieldID": "field_id_TEST",
        "name": "Tax Code",
        "orderWeight": 1,
        "type": "text",
    },
    field_id_ANOTHER_ONE: {

        "defaultValue": "option A",
        "fieldID": "field_id_ANOTHER_ONE",
        "name": "Project",
        "orderWeight": 2,
        "type": "dropdown",
     	"values": ["option1", "option2", "option3"]
    },
    field_id_DATE_FIELD: {
        "defaultValue": "2023-11-19",
        "fieldID": "field_id_DATE_FIELD",
        "name": "Effective Date",
        "orderWeight": 3,
        "type": "date",
    }
})

Enable canUseReportFields beta in Permissions.ts file.
Verify that the custom fields show up.
Click on each of the field.
Enter a value and save it.
Make sure that the field gets saved successfully.

[x] Verify that no errors appear in the JS console

Offline tests

Same as above
QA Steps

In Old dot, create a new Collect / Control policy.
Go to the settings of this workspace and execute the following in console:

p = Policy.getCurrent();
p.policy.isPolicyExpenseChatEnabled = "true";
p.save();

Go to the reports section of the settings of the newly create workspace and add some report fields.
Invite a user to be a member of this new workspace.
In NewDot, login as the invited user. Make sure canUseReportFields beta is enabled for the user.
Go to the workspace chat room and request some money.
Open the created expense report.
Verify that the custom fields show up.
Click on each of the field.
Enter a value and save it.
Make sure that the field gets saved successfully.

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android: Native
[x] Android: mWeb Chrome
[x] iOS: Native
[x] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

https://github.com/Expensify/App/assets/30054992/3ad169a9-d2bc-4430-a263-69c5a70b0a86

iOS: Native

https://github.com/Expensify/App/assets/30054992/5aac6c61-98a2-42d0-a433-df9e016279af

iOS: mWeb Safari

https://github.com/Expensify/App/assets/30054992/8c45f3e0-abfe-4073-b79f-cd917b3337a9

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/30054992/18e2c872-1a80-4006-8a49-316af1cd9d0a

MacOS: Desktop

https://github.com/Expensify/App/assets/30054992/32f0f850-70c1-49bd-93bb-abf9ec07c049

Reviewer Checklist

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[ ] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[ ] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If any new file was added I verified that:

[ ] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[ ] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[ ] If the PR modifies the form input styles:

[ ] I verified that all the inputs inside a form are aligned with each other.
[ ] I added Design label so the design team can review the changes.

[ ] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[ ] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

@allroundexperts Could you add an Android native screenshot?

@jjcoffee I am having issues with Android build. I think testing on web should be sufficient, no?

@allroundexperts Testing well! I guess the relevant changes for OpenReport aren't live yet as signing out and back in resets the changes?

@allroundexperts Testing well! I guess the relevant changes for OpenReport aren't live yet as signing out and back in resets the changes?

That's correct!

I will make an issue so that openReport returns the report fields - I thought it returned all report NVPS but let me double check

Also while we're here, could we also update the Report.ts to include a new key called reportFields which is a json keyed by reportfieldIDs like below?

@allroundexperts There seems to be quite a delay for updating the field value on Android native only, compared with updating a request:
https://github.com/Expensify/App/assets/27287420/29551893-4153-4e7d-b168-5474812152f0

Also while we're here, could we also update the Report.ts to include a new key called reportFields which is a json keyed by reportfieldIDs like below? I have a PR up that passes these down

@thienlnam That exists already. One thing to note is that for the reports, we don't really need the full reportField object. That object exists in the policy already so I don't see any point in duplicating that into reports everywhere. We should just store the value of the field in the report.

This doesn't seem to have anything to do with the code of this PR. I think we can ignore this here.

Agreed, I guess it's an Onyx issue. I retested on a smaller report and it's much faster.

One thing to note is that for the reports, we don't really need the full reportField object. That object exists in the policy already so I don't see any point in duplicating that into reports everywhere. We should just store the value of the field in the report.

We do this because we store the report fields at the time the report was created. For example, if you had some report fields, all expense reports would use those fields.
If you decided to change all of your report fields, you don't actually want it to change on your previous submitted reports. Those will retain the report fields that were already set. If we only had the object in the policy, we wouldn't have a way to see the old set report fields on an older report

I thought we discussed this at some point and decided to use the fields stored in the policy as a single source of truth. However, if we need to change that, then the backend will need to ensure that for an open report (one that hasn't been settled), the fields attached to a report are in sync with the fields that are attached to the policy.
Given this, do you think I should do the changes in this PR or as a separate issue?

I thought we discussed this at some point and decided to use the fields stored in the policy as a single source of truth. However, if we need to change that, then the backend will need to ensure that for an open report (one that hasn't been settled), the fields attached to a report are in sync with the fields that are attached to the policy.
Given this, do you think I should do the changes in this PR or as a separate issue?

The fields stored in the policy are the single source of truth for all new and un-submitted reports on a policy - but for already submitted / approved reports, the report fields remain unchanged otherwise any report field change on the policy would require you to go back and update old reports.

However, if we need to change that, then the backend will need to ensure that for an open report (one that hasn't been settled), the fields attached to a report are in sync with the fields that are attached to the policy.

Yes this is a good point. For all created but open reports, if a report field is changed then we will need to make sure the report has that field. Any policy change to the report fields will update the policyReportFields onyx key - so that can be the source of truth for what fields to show on an open report. For non-open reports however, the report fields should remain unchanged by using the report fields from report.reportFields. So depending on the state of the report, we show different fields. We could do this on the BE by ensuring that every report.reportFields is correct but then when we update a report field, we're updating every single open report for the entire policy as opposed to just updating the policyReportField

@jjcoffee There's been a lot that has changed in this PR since your original review, could you please do another review / quick round of testing just to make sure none of the changes add any blockers?

Additionally, @allroundexperts could you please update the QA steps with the these instructions https://github.com/Expensify/App/pull/34889#issuecomment-1911520656 so that applause can test this when it goes to staging?

Additionally, @allroundexperts could you please update the QA steps with the these instructions #34889 (comment) so that applause can test this when it goes to staging?

Sure thing. I'm done with my day so will take care of it tomorrow!

@thienlnam Testing well so far! Just wondering if the OpenReport fix for setting the fields has hit production yet? None of the changes seem to persist, e.g. on navigating to another report or refreshing.

Not sure if this is within scope of this PR, but trying to save the text_title field results in a You can't rename the report of a free plan 666 error from the BE, which isn't displayed anywhere on the FE.

@allroundexperts Bug: Open any report (without doing any of the Onyx.set steps), enter some value for the VAT Number default field returned from BE.  The field values disappear. (I assume this is related to the BE changes not being live).
https://github.com/Expensify/App/assets/27287420/05385a4d-feb6-4531-95ec-2f7ca11cbd07

Bug? Going in to edit the default title returned by the BE I'd expect this to be an editable formula.

@thienlnam Testing well so far! Just wondering if the OpenReport fix for setting the fields has hit production yet? None of the changes seem to persist, e.g. on navigating to another report or refreshing.

OpenReport should be returning the fields under report.reportFields, I'm not aware of anything being broken with it, but I also have some pusher updates changes for Report_SetFields

Not sure if this is within scope of this PR, but trying to save the text_title field results in a You can't rename the report of a free plan 666 error from the BE, which isn't displayed anywhere on the FE.

I think this because we're trying to add reportFields on a free plan for testing here - in theory, you won't be able to add these for free plans

I agree with @thienlnam about all these bugs.

Added the correct QA steps @thienlnam. Please let me know if they look good enough.

Yup - the QA steps look solid

@allroundexperts Bug: Open any report (without doing any of the Onyx.set steps), enter some value for the VAT Number default field returned from BE. Note several console errors:

Have you been able to check this bug?

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

🚀 Deployed to production by https://github.com/marcaaron in version: 1.4.36-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅


Distance Measurement

Hi,
What is the reference for measuring distance to objects by camera? and how do you estimate it?
Please, help me. Thanks

We didn't measure distance to objects by camera.
On Sun, Dec 11, 2022 at 3:24 AM hesamira @.***> wrote:

Hi,
What is the reference for measuring distance to objects by camera? and how
do you estimate it?
Please, help me. Thanks
—
Reply to this email directly, view it on GitHub
https://github.com/google-research/pix2seq/issues/27, or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAKERUN2OARNOX2SSSND4QDWMW2XTANCNFSM6AAAAAAS25DNBY
.
You are receiving this because you are subscribed to this thread.Message
ID: @.***>

Reference is a fixed point, based on which the distance to objects is measured. For example, your reference here can be the camera.
Did you consider the camera fixed or moving?
Did you calibrate the camera? Do you need to calibrate every time you use the camera or did you just calibrate once?

We follow the standard object detection setup, in which you're given just
images without any camera intrinsics. It may be helpful to check out the
COCO dataset.
On Mon, Dec 12, 2022 at 10:18 PM hesamira @.***> wrote:

Reference is a fixed point, based on which the distance to objects is
measured. For example, your reference here can be the camera.
Did you consider the camera fixed or moving?
Did you calibrate the camera? Do you need to calibrate every time you use
the camera or did you just calibrate once?
—
Reply to this email directly, view it on GitHub
https://github.com/google-research/pix2seq/issues/27#issuecomment-1347803267,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAKERUN57D5ZGLW2GT62WRLWNAILRANCNFSM6AAAAAAS25DNBY
.
You are receiving this because you commented.Message ID:
@.***>


Downgrade Cura.app to v15.04.2 (latest stable)

The previous Cask referenced a beta version.
Now the latest stable version is in use.
Please see homebrew-versions 'cura-beta.rb' for latest beta.

Thank you for the contribution. It was merged directly as ccaf00a189cdbe50109ba6bf294c2b72aecba784 to keep commit history cleaner. Your contribution is still credited to you.


Update uses of EXPLAIN(OPT) to reflect simpler default output

Via @andy-kimball comment on #4340:

The default output of EXPLAIN(OPT) has changed to show much less detail. To show more verbosity, you can use EXPLAIN(OPT,VERBOSE) or EXPLAIN(OPT,TYPES). You'll need to update the sample output accordingly.

Estimated scope of work:

Cost-based optimizer (the "View query plan" section)
EXPLAIN (OPT) docs
Possibly other examples

Update: The EXPLAIN portion of this has been accomplished in #4579 .


ModuleNotFoundError: No module named 'app'

While running some of the steps mentioned in the README, i encountered module not found error.
Should this get fixed?
PS: I updated my PYTHONPATH to fix it, but thought of mentioning it for others.

Thanks for documenting this issue for others! Closing for now but should still be searchable within the issue history for others to find it.


Create workflow to release alpha version on PR Merge

A workflow that will run and release an alpha version each time a PR is merged into main. Don't know how f-droid handles that, but we can refer some apps on fdroid.

Version must be upgraded automatically and changelog has to be added in fastlane directory.

There is no way to release alpha or any other versions on fdroid. Since F-droid doesn't support alpha/beta/prod.


Update timestamps/countdowns in background

Right now if you have a page open and something shows "in 3 minutes", this won't update as time goes by. Would be nice to update every couple minutes without causing too much re-rendering.

This is needed for showing timestamps on comments on livestreams.
At least add for that use case vs everywhere else (claim tiles)


New linter rule: use-recent-module-versions

Fixes #13553
Most changes are packages.lock.json and tests

PublicRegistryModuleMetadata* -> moved to Bicep.Core
IPublicRegistryModuleMetadataProvider -> changed so that it returns only what's currently in the cache at a given time (although it'll start a cache update in the background).
Linter rules can now access an IServiceProvider
New linter rule does not download module metadata info itself, but just uses current cache.  Will display a warning until the cache is downloaded.
PublicRegistryModuleMetadataProvider worked into the artifact restoration process of OCI artifacts (as discussed in Bicep meeting). This way, the linter rule will not hold up compilation, and uses can make sure the cache is not downloaded using --no-restore (although in the current iteration there's no cache on disk so it will give an error in that mode from CLI - this could change in the future).
Only handles public modules for now, but is designed to be able to deal with private registries later

Microsoft Reviewers: Open in CodeFlow

@shenglol Thanks!


QueryRoutes always returns same route

I'm using 0.7.0-beta-rc1 with the latest GRPC files. For my rebalance script at https://github.com/C-Otto/rebalance-lnd I need to try a second route if the first does not work out due to technical reasons (not enough funds, most likely). It's also possible that a route looks fine, but is considered too expensive (so no payment is attempted).
With https://github.com/lightningnetwork/lnd/pull/2497 and https://github.com/lightningnetwork/lnd/commit/7a5bd29a69d659629f8fdcc8dd9aec7b2cfa4904 it's not possible to request more than a single route.
However, in my experiments QueryRoutes always returns the same route, even if sending a payment fails. I'd prefer lnd to give another route when running the query a second time.
A crucial contributor to this might be that the requested/returned route is modified before sending the payment (by adding a lost hop routing back to sending node, to rebalance a channel). If there are heuristics that (temporarily) block hops, these might have to be tuned.
The discussion in https://github.com/lightningnetwork/lnd/pull/2497 mentions "a follow up PR will add any source to destination queries to QueryRoutes", which sounds promising. Please point me to any information regarding this, if it exists.

In my rebalancing script (https://github.com/bitromortac/lndmanage), I'm checking the error code for the failed channel id and then put it on the channel blacklist using the "ignored_edges" field. In that way lnd returns an alternative route.

I know this is not a solution, but it is my understanding that the rationale for this change is that you are encouraged to use the failure as something useful which you can feed to your routing algorithm, to get "better" queries in the future, similar to what @bitromortac says he is doing above.

@willcl-ark I don't have a routing algorithm, I'd like lnd to figure out the details of how to reach the destination.

Currently queryRoutes doesn't use the new mission control system, which would otherwise inform its subsequent routes
The old result was a partial solution but it also wouldn't necessarily return substantially different routes, even if you saw 100 routes they could all potentially be failing in the same way

I think using the ignored_nodes and ignored_edges feature of QueryRoutes is the best option. @alexbosworth is there an issue that connects QueryRoutes and the mission control system? If not, I'd like to keep this open (or create a more specific issue).

I don't know of such an issue
sendtoroute itself is connected to mission control, so the errors are being recorded to inform the pathfinding of normal payments
I would definitely recommend using ignored_nodes and ignored_edges in conjunction with the getRoutes API as a workaround. I would recommend using the routerrpc sendtoroute RPC to get structured errors which can identify the failing hop and the failing reason.
On intermediate node failures, add the failed hop as an ignored edge to subsequent calls to queryRoutes. In addition you could try adding more ignores for other errors that are not related to the specific hop. For FeeInsufficient, IncorrectCltvExpiry failures, add the forwarding nodes to ignore nodes. On TemporaryNodeFailure errors add the hop forward direction node to ignore nodes, and on the UnknownNextPeer error add the next hop after the failed hop to ignore edges.

Thank you, I'll close this. Please have a look at the new issue.

@alexbosworth could you point me to the structured error information you mentioned? I'm only able to find a string that I'd need to parse. https://github.com/lightningnetwork/lnd/blob/master/lnrpc/routerrpc/router.proto#L145

The Failure message is the structured error, it shouldn't require string parsing

Oh. Thanks :)

If the goal is to replicate the previously available num_routes parameter, there is no need for mission control or sendtoroute.
It would involve implementing the k-shortest path algorithm client-side by using a custom route source and the ignore lists. It should be possible to port over the now removed k-shortest code from lnd.

I see your point. Instead of dealing with graph theory in the small script, I'd prefer to use as much of lnd's internal logic as possible. Ignoring failing edges (or having that feature in QueryRoutes) seems to be easier.

It is a question of who does what. K-shortest proved to be quite intensive in maintenance, at the expense of development of features that we consider more important. The available time is fixed, so we sometimes need to make choices that won't be ideal for everyone.


Add unit test for profilingHandlers

Move deps to openshift/node-observability-agent
Add unit test for profilingHandlers

@Sherine - I pulled the PR locally and executed make test. I noticed code coverage is at about 42% but I suppose this is the initial commit for the unit tests.
Other than that it looks good.

@Sherine - I pulled the PR locally and executed make test. I noticed code coverage is at about 42% but I suppose this is the initial commit for the unit tests.
Other than that it looks good.

Yes! this is the first batch . Another one coming up tomorrow hopefully.


CartoonFace

This CV2 library is used to cartoonify the images of normal persons
Related Issues or bug

Info about Issue or bug

Fixes: #[issue number that will be closed through this PR]
Proposed Changes

Info about changes Goes here

Additional Info

Anything related Issues

Screenshots

Original
Updated

original screenshot
**updated screenshot **

closing the pr


Taco Pie: a TCP Client & Server C++11 library.

@atkawa7,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

LGTM, thanks!


QoS - API and implemention for event callbacks

Depends on ros2/rcl#408
Expose QoS event callbacks to the ROS2 Python client.

For folks looking at this here's a demo app i've been using to try it out
import rclpy
# from rclpy.duration import Duration
from rclpy.executors import SingleThreadedExecutor
from rclpy.node import Node
from rclpy.publisher import PublisherEventCallbacks
# from rclpy.qos import QoSLivelinessPolicy
from rclpy.qos import QoSProfile
from rclpy.subscription import SubscriptionEventCallbacks

from std_msgs.msg import String

class Talker(Node):
    def __init__(self):
        super().__init__('Talker')
        qos = QoSProfile(
            # lifespan=Duration(seconds=0),
            # deadline=Duration(seconds=0),
            # liveliness=QoSLivelinessPolicy.RMW_QOS_POLICY_LIVELINESS_AUTOMATIC,
            # liveliness_lease_duration=Duration(seconds=2),
        )
        self.publisher = self.create_publisher(
            String, 'topic', qos_profile=qos,
            event_callbacks=PublisherEventCallbacks(
                deadline_callback=self.dead_cb,
                liveliness_callback=self.live_cb
            ))
        self.count = 0

        self.timer = self.create_timer(0.6, self.timer_cb)

    def fini(self):
        # Destroy the timer attached to the node explicitly
        # (optional - otherwise it will be done automatically
        # when the garbage collector destroys the node object)
        self.get_logger().info('Shutting down')
        self.destroy_timer(self.timer)
        self.destroy_node()

    def timer_cb(self):
        msg = String()
        msg.data = 'Hello World: %d' % self.count
        self.count += 1
        self.get_logger().info('Publishing: "%s"' % msg.data)
        self.publisher.publish(msg)

    def dead_cb(self, evt):
        self.get_logger().info('DEADLINE missed: "{}"'.format(evt))

    def live_cb(self, evt):
        self.get_logger().info('LIVELINESS failed: "{}"'.format(evt))

class Listener(Node):
    def __init__(self):
        super().__init__('Listener')
        self.subscription = self.create_subscription(
            String, 'topic', self.msg_cb,
            event_callbacks=SubscriptionEventCallbacks(
                deadline_callback=self.dead_cb,
                liveliness_callback=self.live_cb
            ))

    def msg_cb(self, msg):
        self.get_logger().info('Heard: "%s"' % msg.data)

    def dead_cb(self, evt):
        self.get_logger().info('DEADLINE missed: "{}"'.format(evt))

    def live_cb(self, evt):
        self.get_logger().info('LIVELINESS changed: "{}"'.format(evt))

def main(args=None):
    rclpy.init(args=args)

    executor = SingleThreadedExecutor()

    talker = Talker()
    listener = Listener()

    def killa():
        executor.remove_node(talker)
        talker.fini()

    killswitch = talker.create_timer(4.0, killa)  # NOQA

    executor.add_node(talker)
    executor.add_node(listener)

    executor.spin()

    talker.fini()
    rclpy.shutdown()

if __name__ == '__main__':
    main()

@thomas-moulard I think this is ready (except for one last TODO that I am tracking down) it seems like you've got a bit of experience with C-Python interfaces, can you give this a once-over review before I open up the review to openrobotics?

this PR needs unit tests.

Working on adding some unit tests still - but @thomas-moulard do you like the look of it better this way? I tried to make the functions more readable, and I see the value in abstracting away some of the string-based functions, so that those potential mistakes can at least be centralized.

Closing in favor of https://github.com/ros2/rclpy/pull/316


Add copy explaining photography containing copywrighted works

Problem

On occasion CC-licensed works may contain photographic subjects—buildings, artworks, or corporate logos, for example—which are, in fact, copywrighted.
Description
We should add language to Openverse which clarifies how to handle these types of work. Here is the proposed copy to add to the "How to use" section below the "Visit the {medias} website" paragraph:

Photographs may contain copyrighted works (for example, paintings, sculptures or architectural works). The use of photographs depicting copyrighted works may require additional permissions from the copyright holder of those works.

This paragraph should conditionally render only on image results.

Alternatives

Additional context

I would like to work on this issue

I don't think we have a design for this. @fcoveram can confirm but in the meantime I'll apply a "design: needed" label.
@HarshYadavade please pick another issue to work on till we have design mockups. Once we have those, you can work on it.

Thanks for the ping. I don't think this ticket requires design, as the new paragraph should follow the existing style. However, I will share this ticket in the marketing channel asking for more eyes on the copy.

The suggested text sounds good to me! However, here are a few suggestions for minor edits that I think can enhance its readability and make it easier for users to grasp the message quickly:

Some photographs might contain copyrighted content, such as paintings, sculptures, or architectural works. Use of such photographs may require additional permissions from the copyright holder of the depicted works.

Or

Some photographs might contain copyrighted content, such as paintings, sculptures, or architectural works. Using these photographs may require additional permissions from the copyright holder of the depicted works.

Thanks @rmartinezduque, I like the second option.

Some photographs might contain copyrighted content, such as paintings, sculptures, or architectural works. Using these photographs may require additional permissions from the copyright holder of the depicted works.

Replacing the design needed label with the ready for work.

@dhruvkb @zackkrida  I'm interested in working on this issue. May I take it up?
Thanks!

@lucasferrazlima we're currentl in a frontend code freeze to migrate to Nuxt 3 so frontend contributions cannot be merged. In the meantime would you like to work on something related to the catalog, ingestion server or API instead?

@dhruvkb Oh got it! Thanks for the update. Yes, i would also like to work on these. I'll check for some other interesting discussions.

Thank you very much, I'll update here once the freeze is lifted.

Hi @lucasferrazlima, our frontend code freeze is lifted if you're interested in revising this work.
If not, that is also fine, and I can take on this PR this week.

Hey @zackkrida, thanks for letting me know!
You can take the PR.


Document how to add haddocks to derived instances

This is how:
data Foo = Foo
  deriving Eq -- ^ Documenting a single type

data Bar = Bar
  deriving ( Eq -- ^ Documenting one of multiple types
           , Ord
           )

Maybe worth also adding an example with DerivingVia?
{-# LANGUAGE DerivingVia #-}
module Via where
import Data.Monoid

newtype Foo = Foo Int
  deriving Show         -- ^ show
  deriving ( Monoid     -- ^ monoid
           , Semigroup  -- ^ semigroup
           ) via (Sum Int)


JavaScript Action

Thanks a lot for great library. I'm trying to use it with a web site link, but when click some links I don't get any action. Then an action sheet is displayed with title "JavaScript" and two buttons "Open" and "Cancel". I have used "Open" button, but nothing happened. I would appreciate your help.

Seems like there's something wrong with your javascript code or that this is something that's unsupported by embedded UIWebViews.


elm-get: docs: getDirectoryContents: does not exist (No such file or directory)

I'm messing around to see if I can break elm-get.
Here's a way:

write a nice elm_dependencies.json file.
Add empty elm files.
Try elm-get publish

The result is: elm-get: docs: getDirectoryContents: does not exist (No such file or directory). I would prefer a more graceful error message, but it's not a big deal.

Is this still an issue with elm-package? If so, lets open an issue that has a complete and minimal example that reproduces the behavior, ideally as a gist that anyone can download as a .zip and dig into.


Default prop value is not reflected in custom element property

Vue version
3.3.4
Link to minimal reproduction
https://stackblitz.com/edit/vitejs-vite-jpamc8
Steps to reproduce
Two issues with props inside elements defined with  defineCustomElement:

vue component respects default value configured with ie. defineProps but it's not reflected in host property value on creation
setting value on custom element and removing it afterwards strips default value and returns null instead

What is expected?

prop's defaults are reflected on a host element
prop's defaults are used as a fallback in case value was removed on a host

What is actually happening?

undefined is returned instead of prop's default
null is returned instead of prop's default once ie. host's value attribute was set and then removed by a user

System Info
No response
Any additional comments?
This is troublesome with form inputs where inputs are expected to return empty string '' as no value and never returns null (they are actually always converted to '' so el.value = null results in value being an empty string).

Just rename the prop from on the components.
In HelloWorld.vue rename hello to value
OR
In MyCustomElement.vue rename value to hello.

Sorry, hello property was some leftover - I just removed it from repro.

#9012 has fixed the second issue, i.e. prop's defaults are used as a fallback in case value was removed on a host.
But the first one (prop's defaults should be reflected on a host element) is still present.

#9012 has fixed the second issue, i.e. prop's defaults are used as a fallback in case value was removed on a host. But the first one (prop's defaults should be reflected on a host element) is still present.

😄
I re-file a new PR to handling the remaining issues