Overhaul README: Wording etc.

@rinigus, if you have questions, suggestions, requests for adaptions etc., please let us discuss them first here.
You as the principal maintainer of SailfishOS:Chum implicitly have the final say ("last word") on anything happening in this repo, but OTOH you know by now, that I set (almost) every character in a text deliberately.  While switching off "maintainer commits" would be childish (and futile), I ask you not to use them without discussing first.

Started reading it and would be happy to discuss. OK, this is then the first PR we have to handle, before #49 . Much of docs has been handled by @piggz, pinging him as well :)

One last change I would make ... add a link and description of adding the chum store meta data to the rpm .spec file

Thank you for revising the README. All in all, I think text has improved.

Good.

Almost all suggested changes are removal of forced HTML linkebreaks.

I see, though I tried to use them sparingly.
IMO they are less awkward than the strange tricks used before:

Using a single line unordered list to achieve a forced line break.
Using a single line pre-formatted text block to achieve a forced line-break.

Furthermore, the looks are really awkward at some other places without forced line breaks:

The FAQ questions look really strange, if not solitary on a line, i.e., if the answer is directly appended to it.
But an empty line breaks a list element into a list element and an independent line outside of the list.
URLs can span over a line end (at least I saw that in the preview): It makes them harder to see, click and comprehend that it is not two URLs, if they are not within a single line.
Sometimes it just makes sense to emphasise a sentence within a paragraph by letting it begin at the start of a line.  Usually because it is important and shall not be quickly read over.  Breaking the paragraph into two breaks the logical cohesion between the sentences, hence this is not a "solution", because it significantly alters the structure and meaning of a text.

In know well, that one should not try to force a HTML renderer into one's own ideas of formatting (because the easily backfires), and this is not what I want to achieve here.  But your apparently strict policy of never using a <br /> is hard to comprehend for me in above cases, especially when you suggest workarounds which definitely fall into the category "but that is not how it was intended to be used" (technically in the first two examples, semantically the other three).
Can you please briefly denote, where your strong aversion of line-breaks originates.
At least I now understand why you repeatedly felt the urge to either eliminate the single line-breaks or convert them into a double line break when submitting my suggestions from issue #45.
BTW (& JFYI), I eliminated two forced line-breaks in PR #50: I avoid them when easily possible.

You are also using nbsp; in the text.

I wish Jolla would have never altered the spelling from "SailfishOS" (before 2015) to "Sailfish OS".  I usually ignore this change, but your "ambition is to become the principal software distribution for Sailfish OS".  So I expected some appeasement to Jolla's spelling rules to be necessary.

One last change I would make ... add a link and description of adding the chum store meta data to the rpm .spec file

Done.

The FAQ questions look really strange, if not solitary on a line, i.e., if the answer is directly appended to it.
But an empty line breaks a list element into a list element and an independent line outside of the list.

Notice that this is a location where I haven't suggested to drop breaks.

URLs can span over a line end (at least I saw that in the preview): It makes them harder to see, click and comprehend that it is not two URLs, if they are not within a single line.

In practice, you just click at URL and focus on the next text rendered after clicking on it. I wouldn't worry about it - although it could have been easily my own hacky "workaround".

Sometimes it just makes sense to emphasise a sentence within a paragraph by letting it begin at the start of a line.  Usually because it is important and shall not be quickly read over.  Breaking the paragraph into two breaks the logical cohesion between the sentences, hence this is not a "solution", because it significantly alters the structure and meaning of a text.

Here I would oppose it. If it is within the paragraph then it should just be in there. If there is something important in the middle of the paragraph that you don't want to miss it then we have to either rewrite the paragraph and add that message in front or split the paragraphs into two. Having a linebreak within the paragraph, in my opinion, is not acceptable when we have a regular text. We can look through many texts from different backgrounds and it is rather common approach throughout.
So, please, if you wish to use a linebreak, unless we have such specific case as FAQ, then either decide whether it is new paragraph or skip it.
I guess the text above responds to your more general question regarding linebreaks.

I think this is OK as MarkDown is missing such feature as non-breakable-space, as we have in few other places. Just we will probably forget to add it and happily let it break there.

Thank you very much for the changes! LGTM

@rinigus,

The FAQ questions look really strange, if not solitary on a line, i.e., if the answer is directly appended to it.
But an empty line breaks a list element into a list element and an independent line outside of the list.

Notice that this is a location where I haven't suggested to drop breaks.

Well, https://github.com/sailfishos-chum/main/pull/48#discussion_r785462090.
Thus they are gone now.

URLs can span over a line end (at least I saw that in the preview): It makes them harder to see, click and comprehend that it is not two URLs, if they are not within a single line.

In practice, you just click at URL and focus on the next text rendered after clicking on it. I wouldn't worry about it - although it could have been easily my own hacky "workaround".

See https://github.com/sailfishos-chum/main/blame/72f41cb65626ad16255d6a7d0863496ca7762036/README.md#L23.
Later only the URL was adapted by flypig from build.merproject.org to build.sailfishos.org, until https://github.com/sailfishos-chum/main/pull/48#discussion_r785457642.
But the "hacky workaround" and my forced line-break are gone now.

Sometimes it just makes sense to emphasise a sentence within a paragraph by letting it begin at the start of a line.  Usually because it is important and shall not be quickly read over.  Breaking the paragraph into two breaks the logical cohesion between the sentences, hence this is not a "solution", because it significantly alters the structure and meaning of a text.

Here I would oppose it. If it is within the paragraph then it should just be in there. If there is something important in the middle of the paragraph that you don't want to miss it then we have to either rewrite the paragraph and add that message in front or split the paragraphs into two. Having a linebreak within the paragraph, in my opinion, is not acceptable when we have a regular text. We can look through many texts from different backgrounds and it is rather common approach throughout.

To me this is a style question and there also are many texts handling this "my way", thus I am still surprised that you think of "Having a linebreak within the paragraph, in my opinion, is not acceptable when we have a regular text.", because "not acceptable" is a very strong rsp. strict stance.
Anyway, we gained some mutual understanding, which is a good outcome from my POV.

Looking at the README as rendered at the sailfishos-chum/main frontpage now, made me wish to re-insert forced line-breaks at least at the locations where @rinigus indicated that they are acceptable for him.
Thus when I spotted a typo, I took the chance: PR #51

@Olf0: agree about strong wording - should have used some lighter tone. Sorry about it.

@Olf0: agree about strong wording - should have used some lighter tone. Sorry about it.

Socially that was O.K. from my perspective, thus no need to say "sorry": You expressed your opinion WRT "forced line breaks" very clearly, but it never was directed "ad hominem".
And I always prefer unambiguous statements over soft, ambiguous ones: "Clarity is paramount!"
Hence I do not think you "should have used some lighter tone", if that was your opinion at that time.
But I am very glad that you seem to have "lightened" the strength of your opinion a bit, because in my understanding your original stance left no room to apply "forced line breaks" at all, which conflicts with my belief that they are sometimes (although rarely) necessary.
Thus gaining some mutual understanding and seeing that PR #51 apparently was O.K. for you is a proper base for future contributions from my perspective.
Thanks!
P.S.: Actually I have one file on my radar, which contains "forced line breaks" at places where even I do not think they are appropriate.  A PR from me for fixing that might come some time.

I am sure we can strike a balance, as we did with PR #51. Would just have to be sure that the break is, indeed, very needed there.


automatic break inside the switch statement

Hi,
coffee-script compiles this
loop
  switch step
    when 1
      console.log "do it and continue with step 2"
    when 2
      console.log "do it and in some situation skip step 1"
    when 3
      console.log "do it"

to that
while (true) {
  switch (step) {
    case 1:
      console.log("do it and continue with step 2");
      break;
    case 2:
      console.log("do it and in some situation skip step 1");
      break;
    case 3:
      console.log("do it");
  }
}

But I don't want the break inside case 1. What can I do?
-Cheers Alex

Do you mean this?
loop
  switch step
    when 1, 2
      console.log "do it and continue with step 2"
    when 2
      console.log "do it and in some situation skip step 1"
    when 3
      console.log "do it"

->
while (true) {
  switch (step) {
    case 1:
    case 2:
      console.log("do it and continue with step 2");
      break;
    case 2:
      console.log("do it and in some situation skip step 1");
      break;
    case 3:
      console.log("do it");
  }
}

No, I want coffee-script to produce this:
while (true) {
  switch (step) {
    case 1:
      console.log("do it and continue with step 2 without break");
    case 2:
      console.log("do it and in some situation skip step 1");
      break;
    case 3:
      console.log("do it");
  }
}

To avoid javascript of jumping around, but still be possible to jump to case 2 and skip case 1

loop
  switch step
    when 1, 2
      console.log "do it and continue with step 2"
      console.log "do it and in some situation skip step 1" if step is 2
    when 3
      console.log "do it"

... or whatever. Fall-through in switch statements is a serious anti-pattern -- not so easy to read. CoffeeScript explicitly wants to prevent you from writing it ;)

$ coco -bps
  for ever
    switch step
    case 1
      console.log "do it and continue with step 2"
      fallthrough
    case 2
      console.log "do it and in some situation skip step 1"
    case 3
      console.log "do it"

for (;;) {
  switch (step) {
  case 1:
    console.log("do it and continue with step 2");
    // fallthrough
  case 2:
    console.log("do it and in some situation skip step 1");
    break;
  case 3:
    console.log("do it");
  }
}

Coco allows the pattern because it can

be readable.
result in DRY code.
help porting JS.

anti-pattern, maybe. But not in all cases:
Just imagine a FSM with three states: INIT, READ, WRITE
var state;

state = "INIT";

while (true) {
  switch (state) {
    case "INIT":
      console.log("do some init steps");
      console.log("fall-through to READ, no matter what they say");
    case "READ":
      console.log("read until you have to write");
      break;
    case "WRITE":
      console.log("write until you have to read");
      if ("you are brave enough") {
        console.log("Reset sometime by switching to INIT");
      }
  }
}

The fall-through is in my opinion totally legitimate and straightforward implementation of this FSM as I want to READ after INIT, in all cases!
And it has also a good readablity if you think on the associated FSM graph.
What do you think?

I think that requirement of break statement inside switch-cases is a cancer of all C-like programming languages.
Moreover, I think that break-s and continue-s are harmful in loops too (@douglascrockford thinks the same about continue statements). Maybe that's because I aim to write CoffeeScript code in functional style, with a lots of pure functions. This helps a lot in debugging and testing, though.

it can be readable.

case 1
  console.log "do it and continue with step 2"
  fallthrough
case 2
  ...

Nope. At least not for me.

Moreover, I think that break-s and continue-s are harmful in loops too. Maybe that's because I aim to write CoffeeScript code in functional style, with a lots of pure functions.

Maybe you should move on. There are much better alternatives if you prefer functional approach.

@alexwall your example can be rewritten with one simple function call inside init().
state = 'INIT'

write = ->
  console.log 'Writing'

read = ->
  console.log 'Reading'

init = ->
  console.log 'Initializing'
  read()

parseState = ->
  ...

parseState state

@satyr thanks for the link, just noticed Roy, it looks awesome. Though, it's not as widely used as CoffeeScript.

@paulmillr thx for your suggestion, but there are two little problems with this solution.

The control flow of the whole thing I want to implement (in this case the FSM) moves inside the several functions.
It destroys the readability: init() has nothing to do with read()

@alexwall If the INIT step is correctly named, you should just lift it outside the state machine.  If I see an INIT step inside a while loop, that seems like a code smell.

@showell at the very first glance I would agree, but have look on this FSM:

The loop is just to be iterative, so we can do it also the recursive style (which is not so good):
state = "INIT"
 fsm = (state, control) ->
   switch state
     when "INIT"
       console.log "do some init steps"
       nobreak # as it makes no sense here
     when "READ"
       # "read until you have to write"
       if read_counter >= 5
          control = 'write'
       if control == 'read'
         if "no previous errors"
           read_counter += 1
           next_step = "READ"
           read_db_async fsm next_step
         else
          # Restart FSM again
          read_counter = 0
          fsm "INIT"
       else if control == 'write'
         next_step = "WRITE"
         fsm next_step
     when "WRITE"
       console.log "write until you have to read"
       if control == 'end'
          console.log "Successfully reading and writing! Well Done!"

So reading the diagram and the script is just simple and straightforward:
If (when) you are in state X and the control is Y then switch to state Z.

In the iterative version, can't you just set next_state to "READ" and just go back through the loop?  Does it make sense to introduce a new state called "RESET"?

There are no "RESET"-state, I used control='reset' just to describe why I want to switch to back INIT and that INIT is a justified state, which should be inside the loop.
So in respect to the specific characteristics of javascript/node we don't really need a loop to create an iterative version:
interative version

 fsm = (state, control) ->
   switch state
     when "INIT"
       console.log "do some init steps"
       nobreak # as it makes no sense here
     when "READ"
       # "read until you have to write"
       if read_counter >= 5
          control = 'write'
       if control == 'read'
         if "no previous errors"
           read_counter += 1
           next_step = "READ"
           # run an async function and set callback fsm next_step
           read_db_async (-> fsm next_step)
         else
          # Restart FSM again
          read_counter = 0
          process.nextTick (-> fsm "INIT")
       else if control == 'write'
         next_step = "WRITE"
         process.nextTick (-> fsm next_step)
     when "WRITE"
       console.log "write until you have to read"
       if control == 'end'
          console.log "Successfully reading and writing! Well Done!"

 state = "INIT"
 fsm state

So process.nextTick causes that at the next loop around the event loop to call the callback -> fsm next_step. In other words:  process.nextTick (-> fsm next_step) is not recursive function call.
But this is not the point. What I wish is a way to remove an auto generated break.
A nobreak-command would be just fine

fsm = (state, control) ->
  actions =
    INIT: ->
      console.log "do some init steps"
      actions.READ()

    READ: ->
      # "read until you have to write"
      if read_counter >= 5
          control = 'write'
      if control == 'read'
        if "no previous errors"
          read_counter += 1
          next_step = "READ"
          # run an async function and set callback fsm next_step
          read_db_async (-> fsm next_step)
        else
          # Restart FSM again
          read_counter = 0
          process.nextTick (-> fsm "INIT")
      else if control == 'write'
        next_step = "WRITE"
        process.nextTick (-> fsm next_step)

    WRITE: ->
      console.log "write until you have to read"
      if control == 'end'
          console.log "Successfully reading and writing! Well Done!"

  actions[state]()

state = "INIT"
fsm state

Your suggestion seams interessting but it doesn't solve the problem.
My point is: I don't  want unnecessary function calls (break and call fsm again)
Additionally: this solution creates new problems:

inconsistency: actions.READ() to switch to READ-state and using process.nextTick (-> fsm next_step) for other steps
unnecessary workarounds: actions[state](): So switch state is in my opinion a better representation of the fsm
the created javascript code is more complex and has no advantages

So guys show me the light, how can I implement it better in terms of

flexibility
readability
consistent coding style
straightforward implementation of this model (in this case the FSM)

Or please just add a keyword like fallthrough or nobreak. I see no problem for anyone:

adding automatically break will still be the default
older scripts will still be compatible as a fallthrough keyword is just an extension of coffee-script

Use backticks.  http://jashkenas.github.com/coffee-script/#embedded
I don't think the language should necessarily be altered to support such a one-time-use feature, especially when there are so many alternatives even if they aren't quite as optimal.

Using backticks throws me back to javascript, so you have to deal with all the unwanted ballast coffee-script tries to eliminate.
And

one-time-use or not depends your requirements
it's not a feature! Not allowing fallthrough is a limitation of JavaScript

So using your argument style it would be just fine to limit number of parameters a function can take to three, because calling function with four or more parameters is very uncommon, ugly or an anti-pattern.

The point is: You don't have to use fallthrough if you don't like it as it makes no difference to you. So you have completely no disadvantages.
Sometimes I prefer using this
  do something if condition

rather than this
  if condition
    do something

Why can I do this? Because coffee-script intends to be more flexible than JavaScript. This was the first thing I thought as I discovered this language.
You say you want 5 function to do 5 things, ok fine just do it, because you can do it. I want use another structure but coffee-script doesn't allow me. Not allowing  fallthrough is a restriction and each notable language allows this.
In my opinion coffee-script violates at this point its principle to be flexible!

Well, Perl is very flexible. But is it good? I think no. Someone may love perl, but the truth is — most of Perl's code is unmaintainable crap. It is not true for languages like python and ruby, though.
Flexibility / freedom in programming languages is good only until some point.
I don't have to use fallthrough, but, if it would be introduced, most likely I'll have to maintain code with fallthrough's.

Good? Good for what?
Again it depends on your requirements! Are you going to develop an air plain navigation software or just write driver for your web cam. It depends!
So have a look on my "The not smelling state machine"-digram and tell me, is the code really so difficult to read and understand for you?
If somebody doesn’t understand the (mental) model for instance the MVC-Paradigm, then it's certainly difficult to understand the source code. Languages just helps to describe the model. If you don't have proper words you have to explain!
I don't want to explain! I just want to you use a single word. In this case fallthrough!

Well, Perl is very flexible. But is it good? I think no. Someone may love perl, but the truth is — most of Perl's code is unmaintainable crap. It is not true for languages like python and ruby, though.

Unlike Python, Ruby/CoffeeScript falls into the same category as Perl, sharing TIMTOWTDI.

if it would be introduced, most likely I'll have to maintain code with fallthrough's.

Much better than having to maintain embedded JS.

Unlike Python, Ruby/CoffeeScript falls into the same category as Perl, sharing TIMTOWTDI.

Yeah, I know. Still, Ruby / CoffeeScript code is usually much more maintainable. Ruby / coffee guys doesn't write this lot of one-line variable names etc.

Much better than having to maintain embedded JS.

Well, if language prevents user from doing something, I guess it's better not to do this at all.

Languages just helps to describe the model

There's two way to describe the model: simple and complex. fallthrough is complex.
Also (simple / complex) != (easy / hard). Easiness / hardness are relative for everyone. Take a look at great Rick Hickey's (Clojure's author with 20y experience) presentation about simplicity: Simple Made Easy.

+1 for fallthrough, but I guess bury it in the code might be still likely to cause accidental fall-through.
Maybe something like this?
switch state
    case 'INIT'
      console.log "do some init steps and fall through"
    when 'READ'
      console.log "do some read steps"

Whether or not to fall through is not determined by that if the last statement is fallthrough, but by the choice of "branch word" (i.e., case or when).

@gravof nice idea!


incorrect convex hull normal warning

I'm getting (incorrect) warnings saying that the face vertices aren't ordered CCW.
Collisions seem to work though and the faces are rendering properly which, I believe, means that they're ordered correctly. I've tried triangulating the faces but I still get the warnings.
My guess is that the the hull is not centered around the origin and when you compare the faceNormal to the first vertex in the face it is using the world space position of the vertex rather than the delta to the hull's centroid for the comparison.
edit: I've been using these as static rigid bodies and the collisions have been fine, I tried making them active and things get a little crazy.
these vertices and faces trigger a warning:
verts = [
[-264.5525817871094, -20.33197784423828, -13.67792797088623],
[-280.2852783203125, 8.290871620178223, 43.60786056518555],
[118.72366333007812, 61.67375183105469, -183.38092041015625],
[-234.13946533203125, 30.150381088256836, 42.02793884277344],
[-143.5728759765625, 65.76481628417969, 27.467012405395508],
[-205.86732482910156, 98.23126220703125, 2.5629844665527344],
[-382.68603515625, -0.3020574152469635, -223.57937622070312],
[-405.5048522949219, 72.1712875366211, -38.82634735107422],
[-170.37646484375,<PHONE_NUMBER>484375, -156.85577392578125],
[-272.656494140625,<PHONE_NUMBER>0703125, -49.390228271484375],
[-123.59403228759766, -29.31911849975586, -137.66722106933594],
[119.81502532958984, 60.16632080078125, -186.60337829589844],
[-325.28326416015625, -32.679317474365234, -195.95068359375],
[-333.7191162109375, -29.035734176635742, -203.6719207763672],
[-153.84864807128906, 89.69259643554688, -313.27569580078125],
[-366.7395935058594, -3.3212034702301025, -249.88494873046875]
]
faces = [
[1, 7, 6, 13, 12, 0],
[2, 4, 3, 1, 0, 10, 11],
[2, 8, 9, 5, 4],
[5, 3, 4],
[6, 15, 13],
[9, 7, 1, 3, 5],
[12, 10, 0],
[11, 14, 8, 2],
[14, 15, 6, 7, 9, 8],
[12, 13, 15, 14, 11, 10]
]

Figured this out. When I center each hull around the origin and use the offset to position it in world space the warnings go away and everything behaves correctly. I'm doing it procedurally for each hull so maybe it would make sense to incorporate something similar to the source?
var verts = [];
var faces = []

var centroid = new CANNON.Vec3();

positions.forEach( v => {

  var pos = new CANNON.Vec3(v[0],v[1],v[2]);

  centroid.vadd(pos, centroid)

  verts.push(pos)

});

centroid.scale( 1 / verts.length, centroid );

...

polybody.addShape(polyhedron, centroid);


fix: make all postgres table names lowercase

Test Plan
[Describe test plan here]
Deployment instructions
[Add any special deployment instructions here]

We should validate that no two objects write to the same place (i.e. Contact vs contact)

there is a lot of validation we should do. there is a separate task for that on our task board.


Release 1.14.0

I'd like to make a 1.14.0 release soon. @moggers87, ok for that? I'll prepare a commit with Changelog and such in the coming days.

I just pushed the updated translatable strings to Transifex.

#507 is the PR for the Changelog, but #505 and #506 should be merged first, then the translations should be pulled from Transifex.

Pulled the latest translations from Transifex. PR here: https://github.com/jazzband/django-two-factor-auth/pull/508
Once that's been merged we can tag and upload a release

Crap, I tagged and pushed 1.14.0, then realized that we missed bumping version in some places. So I reverted the tag and prepared #509.

Tagged a new release and I think we just wait for CI to do the upload now, right?

Done and done: https://pypi.org/project/django-two-factor-auth/1.14.0/
Thanks @claudep for taking the lead on getting a release out :smiley_cat:

:champagne:


Add synthetic unwind row entry for the function end

Context
DWARF unwind information specifies the low and high PC for every
function it covers. Within a function, it has entries for each
instruction boundary. These entries have the program counter they
start covering, but not its end.
This is not a problem for any instruction boundary except the last one,
but while we have that information when parsing the DWARF info, we never
add it anywhere in the BPF representation.
While this was not a problem for a large majority of the stacks we need
to unwind it introduce two issues:

We know we've reached the bottom frame when a PC doesn't have unwind
info + frame pointer == 0. Normally the bottom frame has a PC that's
not covered by the (lowest, highest) PCs that the unwind info has, but
as far as I know there's no guarantee of this. If we get the last item
of a chunk, we have no way to know if the PC we are interested in is
covered in the unwind info, or not;
The other problem is that we currently have, related to the last point
above, is that we have to chunk some unwind tables when they are too
large for the maximum size of a shard or for the current free space in a
shard. When this happens we might split a function in two. This causes
the same issues as above at every split point.

Proposed solution
This commit adds a fake or synthetic unwind row to indicate the end of a
function. We then handle this appropriately in the unwinder.
Unfortunately we will increase the unwind table size, I expect by ~5-10% on
average, but haven't measured properly. While this is not ideal, we want to
ensure that the statistics we get from the unwinder as are correct as possible.
In the future we might remove some of these markers that aren't totally
necessary (e.g. if we can prove that there can't be any other PC in between
a pair of consecutive rows from different functions. This might be hard
to get 100% right on x86, but could be a possibility).
I was planning on slightly reducing the size of the unwind row, which
should help a bit with the recent size increase from this commit.
Other changes

Made the code that deals with chunking and shard allocation cleaner an
more correct.
The eh-frame utility accepts a new argument to filter unwind info
for a particular function that contains the given PC.

Test Plan

no errors on sudo bpftool prog tracelog | grep err

Co-authored-by: Kemal Akkoyun<EMAIL_ADDRESS>Co-authored-by: Javier Honduvilla Coto<EMAIL_ADDRESS>Signed-off-by: Francisco Javier Honduvilla Coto<EMAIL_ADDRESS>

@javierhonduco I can't officially approve this 😅 Feel free to proceed as you see fit!

That's funny 😄


Adding annotation formula to bar chart add more bars instead of line

Describe the bug
A clear and concise description of what the bug is.
To Reproduce
Steps to reproduce the behavior:

Go to explore
Create time-series bar chart
Add annotation layer
Set y=constant e.g. y=100 as formula

Expected behavior
A horizontal line at y 100
Screenshots
It adds a bar for each grouped bar instead.

Environment (please complete the following information):

superset-ui version: current

@kristw im moving this issue to the main repo. can you post all the future issues there? 🙏

/move to

/move to apache/incubator-superset

https://github.com/apache/incubator-superset/issues/11909


KNOX-2846 - Fix GatewayWebsocketHandlerTest.testValidWebShellRequest

What changes were proposed in this pull request?
Let's see if it fixes this:
2022-11-28T05:34:23.3732998Z [ERROR] testValidWebShellRequest(org.apache.knox.gateway.websockets.GatewayWebsocketHandlerTest)  Time elapsed: 0.585 s  <<< ERROR!
2022-11-28T05:34:23.3733695Z java.lang.IllegalStateException: Unable to load cache item
2022-11-28T05:34:23.3734384Z 	at org.apache.knox.gateway.websockets.GatewayWebsocketHandlerTest.testValidWebShellRequest(GatewayWebsocketHandlerTest.java:88)
2022-11-28T05:34:23.3735487Z Caused by: java.lang.IllegalAccessError: class javax.xml.parsers.FactoryFinder (in unnamed module @0x38954d02) cannot access class jdk.xml.internal.SecuritySupport (in module java.xml) because module java.xml does not export jdk.xml.internal to unnamed module @0x38954d02
2022-11-28T05:34:23.3736670Z 	at org.apache.knox.gateway.websockets.GatewayWebsocketHandlerTest.testValidWebShellRequest(GatewayWebsocketHandlerTest.java:88)

How was this patch tested?

@zeroflag - Could you please explain why those ignore declarations help here? Thanks!

@zeroflag - Could you please explain why those ignore declarations help here? Thanks!

My understanding is that some of those classes are included in the jdk's rt.jar  as well as some third party dependency an powermock loads them from one location while knox loads them from a different location, causing clashes. But it's difficult to unfold all the details. There is lot's of magic going on inside powermock.

I'm rerunning one more time to see how stable it is.


Can't open Inspector Panel under Safari

Description
Under Safari a click to an object do nothing.
Note: The Inspector Panel is only displayed during the tutorial.
What browser are you seeing the problem on? What version are you running?
Safari v17.6 (176<IP_ADDRESS>.7, 17618)
The OS you're using
macOS Monterey 12.7.5 (21H1222)
Steps to reproduce

Zoom to place at level 18
click on an object i.e. Redsqware
Under Chrome the Inspector Panel is displayed, not under Safari.

GERS ID of the entity in question (if applicable)
08f2a134d4d03ad0038b1c8dc878ede0
The browser URL at the time you encountered the bug
https://explore.overturemaps.org/#18.17/39.959201/-75.15175

Thank you for the report @pyrog - unfortunately this looks like it may be hard to repro.
Can you capture the console log in the developer tools and see if you are getting any errors on the click?
I clicked to the location you provided and on the item with the same GERS Id using my mac laptop, and was able to see the inspector panel okay.

A notable difference is that you're using a more up-to-date version of safari, and a different OS than me.


Task/search autos by characteristics

See - https://app.asana.com/0/1202525679663009/1202720801120400/f

Swap if user set min field higher, tnah max field

Plus this PR gained a conflict :crying_cat_face:


Improve the current blocking behaviour with database calls inside transaction context

Description:
ATM by design we block the strand threads when executing database call when it is inside a transaction. IMO we can use seperate thread instead of blocking thread. We need to improve the current logic of handling database code.

This will be depend on language changes in update 9


Dockerization

Implement Docerization and deploy / update scripts.
┆Issue is synchronized with this JIRA Epic
┆Project Name: Boardwalk-UI
┆Issue Number: BOARD-12

➤ Cricket Sloan commented:
This was done as some point, but we are not sure what we need to do in the future.


Option to turn off if-then-else behaviour within patterns

see https://github.com/Schematron/schematron-enhancement-proposals/issues/25

Re-visit if/when group becomes part of the standard.


Decouple AG-reconciler from dynakube-reconciler

Description
ActiveGate reconciler is currently a part of Dynakube reconciler. The latter is explicitly aware of AG types.
Goal of this PR is to separate AG logic from Dynakube reconciler.
AG reconciliation now happens in activegate package.

Extract DynakubeController.reconcileOneAgent() method
Extract createCapability, deleteService and deleteStatefulset from one big if
Move AG reconciler to separate package
Hide AG reconciler internals in internals/ dir to avoid excessive API
AG proxy secret reconciler uses context
Create single reconciler for whole AG logic

dynakube reconciler now needs no knowledge about AG

How can this be tested?

make test/kuttl/activegate (locally succeeded :heavy_check_mark:)

    harness.go:402: run tests finished
    harness.go:511: cleaning up
    harness.go:520: collecting cluster logs to kuttl/activegate/logs/kind-logs-1659966800
    harness.go:568: removing temp folder: "/tmp/kuttl425072543"
    harness.go:574: tearing down kind cluster
--- PASS: kuttl (556.22s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/capabilities-walkthrough (483.58s)
PASS

e2e tests
manual tests with various AG x OneAgent configurations

Checklist

[x] Unit tests have been updated/added
[x] PR is labeled accordingly
[x] I have read and understood the contribution guidelines

currently this is just moving files around, not an improvement in the way the activegate is reconciled?

@chrismuellner the actual reconciliation mechanism didn't change. It's refactored code with added layer that encapsulates AG reconciliation in activegate package instead of making it in dynkaube reconciler.


[NO_JIRA] fix_variable_name

Fix undefined variable name

Since hotfix-4.0.x is closed and I didn't notice this PR was against that branch, I cherry picked this over to develop as 6108adf71f.


Issue finding node module in renderer window process

I'm running into an issue where on reload of a window somewhat rarely the renderer window is not able to resolve a node module and my application is unable to continue running.
This is using a build folder, not an asar file.
Is it possible there are race conditions involved with this code path?
Uncaught Error: Cannot find module 'winston'
Module._resolveFilename @ module.js:332
Module._load @ module.js:282
Module.require @ module.js:361
require @ module.js:380
(anonymous function) @ main.js:29702

process.versions
{
  "http_parser": "2.5.0",
  "node": "2.2.1",
  "v8": "<IP_ADDRESS>",
  "uv": "1.5.0",
  "zlib": "1.2.8",
  "ares": "1.10.1-DEV",
  "modules": "44",
  "openssl": "1.0.2a",
  "electron": "0.28.1",
  "atom-shell": "0.28.1",
  "chrome": "43.0.2357.65"
}

Not sure if related, but fyi with io.js 2.2.0 you get this when you require on Windows with a path that contains unicode characters: https://github.com/nodejs/node/issues/2236

@bpasero thanks for the info, but yes that is likely unrelated.
I haven't a found a way to create a reduced test case, but I have selenium/capybara testing failures related to this that can reproduce this issue somewhat consistently, maybe 30% of the time when the tests are run in a loop.

Are you able to log out require('module').globalPaths when the issue happens?

I should be able to, this error is caught by uncaughtException callbacks so I'll try that in the morning and comment back midday tomorrow.

The global paths appear to be the expected paths. In this case app is symlinked to my build directory, I wouldn't expect that to be a problem.
/Users/aaron.ackerman/Electron.app/Contents/Frameworks/Electron Helper.app/Contents/lib/node
/Users/aaron.ackerman/Electron.app/Contents/Resources/atom.asar/renderer/api/lib
/Users/aaron.ackerman/Electron.app/Contents/Resources/app
/Users/aaron.ackerman/Electron.app/Contents/Resources/app.asar
/Users/aaron.ackerman/Electron.app/Contents/Resources/atom.asar/common/api/lib

I think I have some great info here to begin debugging this problem, I've got two screenshots, the first is a scenario where the module is able to be required with 22 resolved lookup paths.

The second screenshot here, where the module was not able to be required, the app was only able to find 14 lookup paths. To try to reproduce the problem I added a breakpoint to module.js and used a bare require('winston') from the devtools console. Same end result, but I hope that doesn't skew the output.

In the failure case the parent module is coming the atom.asar. In the case where it works, the parent module is the application's index.html file.

@kevinsawicki do you happen to know where I might go from here to try to figure out why the second screenshot error case is possible?

do you happen to know where I might go from here to try to figure out why the second screenshot error case is possible?

I'm not sure, @zcbenz or others might have some ideas.
Are you able to reproduce this issue on electron 0.30+? I wonder if it may be an asar issue that has already been fixed.

I'm having this issue on windows when I load the application over network.
paths like: "\SERVER\stuff\app\resources\app\node_modules"

It is an old bug that has already been fixed, please report back if you are still able to reproduce it on 0.30+.

Would you mind pointing me toward the PR that fixes this issue so I can understand what's happening here?

It was fixed months ago, basically it was caused because the javascript context sometimes is created earlier than the page and Electron failed to add module search paths for the page's location.

I just ran into this with Electron v0.30.6 on OSX 10.10.3.
Here is what I ran from my project root : ./node_modules/.bin/electron .
Here are the results of
// inside index.html
require('module').globalPaths.forEach(console.log.bind(console))

It looks like my apps node_modules folder isnt in the module paths in the renderer process? I would expect to see: /Users/mnichols/dev/pqls/apps/agent-ui/node_modules somewhere in there.
[38631:0831/171832:INFO:CONSOLE(18)] "/Users/mnichols/dev/pqls/apps/agent-ui/node_modules/electron-prebuilt/dist/Electron.app/Contents/Frameworks/Electron Helper.app/Contents/lib/node", source: file:///Users/mnichols/dev/pqls/apps/agent-ui/signIn.html (18)
[38631:0831/171832:INFO:CONSOLE(18)] "/Users/mnichols/dev/pqls/apps/agent-ui/node_modules/electron-prebuilt/dist/Electron.app/Contents/Resources/atom.asar/renderer/api/lib", source: file:///Users/mnichols/dev/pqls/apps/agent-ui/signIn.html (18)
[38631:0831/171832:INFO:CONSOLE(18)] "/Users/mnichols/dev/pqls/apps/agent-ui/node_modules/electron-prebuilt/dist/Electron.app/Contents/Resources/app", source: file:///Users/mnichols/dev/pqls/apps/agent-ui/signIn.html (18)
[38631:0831/171832:INFO:CONSOLE(18)] "/Users/mnichols/dev/pqls/apps/agent-ui/node_modules/electron-prebuilt/dist/Electron.app/Contents/Resources/app.asar", source: file:///Users/mnichols/dev/pqls/apps/agent-ui/signIn.html (18)
[38631:0831/171832:INFO:CONSOLE(18)] "/Users/mnichols/dev/pqls/apps/agent-ui/node_modules/electron-prebuilt/dist/Electron.app/Contents/Resources/atom.asar/common/api/lib", source: file:///Users/mnichols/dev/pqls/apps/agent-ui/signIn.html (18)
[38631:0831/171832:INFO:CONSOLE(336)] "Uncaught Error: Cannot find module 'package.version'", source: module.js (336)

Am I doing something incorrectly?

I do have the same issue. I guess my global paths are wrong, but don't know what to do against that :
require('module').globalPaths
["/home/pi/mediacenter/node_modules/electron-prebuilt/dist/resources/atom.asar/common/api/lib", "/home/pi/mediacenter/node_modules/electron-prebuilt/dist/resources/atom.asar/renderer/api/lib"]

Any clue ?


Allow Vivaria to start if Docker daemon is inaccessible

Vivaria fails to start and serve traffic if the primary VM host's Docker daemon is inaccessible. This PR fixes the issue by catching and logging the error that prevents Vivaria from starting successfully.

Closing because I don't think this is the right way to address this.


Replace cobs implementation

Fixes #130

Go ahead and merge assuming that the varint max size isn't defined by nanopb.

Also, are you planning on adding an option to decode a file, or do we have to use socat?

Also, are you planning on adding an option to decode a file, or do we have to use socat?

Not in this PR. There is also the open data visualisation issue #77 which might, depending on the chosen solution, render reading serial logs to a text format less important.


Manta 99-outputs contains only "dispatched" files

The current (v 2.2) Manta workflow does not symlink any of the actual results from Manta to 99-outputs. Shouldn't at least the bed files be symlinked there or am I misunderstanding the purpose of this directory?
ls results/icgc_dart/manta-2.2/99-outputs/dispatched/genome--hg38/
SP116652--SP116651--matched.dispatched  SP59270--SP59269--matched.dispatched  SP59284--SP59282--matched.dispatched  SP59332--SP59330--matched.dispatched

Did it run into any errors while running? The bed and vcf files should be symlinked into 99-outputs/bed/ and 99-outputs/vcf/ respectively


Add Filter with index for slice #40

#40
relative to #49

slice support
add test case

please check and review it.

That was awesome @mentol310 !


checkbox event.target.checked undefined

Bug, feature request, or proposal:
Bug
What is the expected behavior?
<md-checkbox (click)="test($event)"></md-checkbox>

test(event) {
  console.log(event.target.checked); // true | false
}

What is the current behavior?
<md-checkbox (click)="test($event)"></md-checkbox>

test(event) {
  console.log(event.target.checked); // undefined
}

Which versions of Angular, Material, OS, browsers are affected?
<EMAIL_ADDRESS><EMAIL_ADDRESS><EMAIL_ADDRESS>

Hm, I was looking at this issue, and it looks like the target in the MouseEvent is not the underlaying input
I think this is an issue which is solvable, but would rather make the components more complex than they need.
As a workaround I propose the change event, because it will be triggered when a click was recognized on the underlaying input.

It worked on the eariler version of md-checkbox and the normal <input type="checkbox" (click)="test($event)"> works, so shouldn't the behaviour be the same?

Definitely, the goal is to make the components  as much as consistent to the native ones.
It's just a bit more difficult to forward all click events to the underlaying input, because click events can also happen on the label or other container elements inside of the component.
Waiting for @jelbourn's opinion on this.

The workaround with change is easy to implement in my case, so I'm not sure if this would a problem for someone or not. Just wanted to address this missmatch on normal behaviour and difference in md-checkbox versions :)

@tinayuangao we might need to think a bit about our approach here

I was having the same issue and using (change) instead of (click) works.
I'll leave my plunkr here: http://plnkr.co/edit/QW2UqY6huRtZVqwfmlbU?p=preview
@Component({
  selector: 'demo-component',
  template: `
Value: {{isChecked}}

Native: <input type="checkbox" (click)="doCheck($event)" [checked]="isChecked" />

Click: <md-checkbox (click)="doCheck($event)" [checked]="isChecked"></md-checkbox>

Change: <md-checkbox (change)="doCheck($event)" [checked]="isChecked"></md-checkbox>
`
})
export class DemoComponent {

    isChecked:boolean = false;

    doCheck($event) {
        this.isChecked = !this.isChecked;
    }
}

try this
<input type="checkbox" (change)="selectionChange($event.srcElement)">
selectionChange(input: HTMLInputElementr) {     input.checked === true       ? doSomethingIfTrue()       : doSomethingIfFalse();   }

hello,
try
`<md-checkbox (click)="test($event)">
test(event) {
console.log(event.checked); // event.target.checked
}`

<input type="checkbox" (click)="boxClicked($event)" />

public boxClicked(e: MouseEvent){
    if((HTMLInputElement>e.srcElement).checked) {
        // Do something...
    }
}

try this:   ツ
<mat-checkbox (change)="check($event)"></mat-checkbox>
check(event) {
     console.log(event.checked) // true or false
}

Hi,
you can get checked status true/false using below code.
HTML
<md-checkbox (change)="doCheck($event)">
TS
doCheck(event){
if( event.checked){
console.log('Checked Value Is True');
}else{
console.log('Checked Value Is False');
}
}

How can we get the native event? I have a situation where I want to stop propagation of my event to parent? But I am not able to get that event.

This works properly for me:
<input type="checkbox" (change)="onChecked($event)">

onChecked (e: Event) {
  const checkbox = e.target as HTMLInputElement;

  if (checkbox.checked) {
    // do something
  }
}

Casting to HTMLInputElement works


Remove or decrease the width of Description in Custom fields

Describe the bug
The Description column is too wide, takes up lot of real estate on the page. Due to this other critical pieces of information like Dependency, Usage etc. are at the far right and we have scroll horizontally to see these values. When you scroll and move to this information you no longer see the which field and object the data belongs. Can you please improve this experience.
To Reproduce
Copy and past the information from the error dialog box here!!

I proposed in #230 to shrink the description to up to 50 characters as a start.
But i understand that maybe the right thing to do would be to simply move the description columns as far to the right as it could be....
Would it be good for you?


Outlook integration

Would be really useful to have a Outlook integration. The outlook.js does not seem to work. At least not for the most recent outlook. I was able to get something with the following additions:
integrations.json:
	"outlook.office365.com": {
		"name": "Outlook Mail",
		"link": "*://outlook.office365.com/*",
		"script": "outlook-mail.js"
	},

outlook-mail.js:
// Inbox emails
clockifyButton.render(
	'div[class="MByod XxeQL"]:not(.clockify)',
	{ observe: true },
	function (elem) {
		const description = $('[aria-label="Reading Pane"] [role="heading"]')
		link = clockifyButton.createButton(description.textContent);
		link.style.marginLeft = '20px';
		link.style.fontSize = '14px';
		elem.appendChild(link);
	}
);

This is not pretty but worked for me. I imagine 'div[class="MByod XxeQL"]:not(.clockify)' is not the best, seems like this could change?
I imagine one of you guys could implement a better solution within minutes. I would appreciate that and I am sure many others would to!

Sorry, somehow created this on the wrong repo.


windows 7

I wonder why no one of my games works on windows 7 with ddrawcompat.

It's impossible to say from that much information. Please attach debug logs for a particular game, preferably one that works without DDrawCompat, and has worked in Windows 10 with DDrawCompat (if it was on the same PC).


Increment version for formrecognizer releases

Increment package version after release of azure-ai-formrecognizer

API change check
API changes are not detected in this pull request.

Something blocked this from auto-merging, going to close since I'm adding the changelog and version updates in another PR


Fix cmake config with TestData

Description
Fixes #892. The custom cmake command is invoked at the end of each build: https://stackoverflow.com/questions/34799916/copy-file-from-source-directory-to-binary-directory-using-cmake
How Has This Been Tested?

Screenshots, that help to understand the changes(if applicable):
Checklist:

[ ] A test for the new functionality was added.
[ ] All tests run without failure.
[ ] The new code complies with the TiGL style guide.
[ ] New classes have been added to the Python interface.
[ ] API changes were documented properly in tigl.h.

Codecov Report

Merging #893 (37c6938) into master (e5d4ee8) will decrease coverage by 6.96%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #893      +/-   ##
==========================================
- Coverage   69.30%   62.34%   -6.97%
==========================================
  Files         296      296
  Lines       24750    24750
==========================================
- Hits        17153    15430    -1723
- Misses       7597     9320    +1723

Impacted Files
Coverage Δ

src/wing/CCPACSWingRibsDefinition.h
0.00% <0.00%> (-100.00%)
:arrow_down:

src/geometry/CCPACSGenericSystem.cpp
0.00% <0.00%> (-94.12%)
:arrow_down:

src/geometry/CCPACSGenericSystems.cpp
0.00% <0.00%> (-90.00%)
:arrow_down:

src/CCPACSEtaXsiPoint.cpp
16.00% <0.00%> (-76.00%)
:arrow_down:

src/wing/CCPACSWingSparPosition.cpp
3.66% <0.00%> (-72.48%)
:arrow_down:

.../structural_elements/CCPACSFuselageWallSegment.cpp
4.25% <0.00%> (-70.93%)
:arrow_down:

src/CCPACSCurvePoint.cpp
11.76% <0.00%> (-64.71%)
:arrow_down:

src/wing/CCPACSWingSparSegments.cpp
28.57% <0.00%> (-61.91%)
:arrow_down:

src/wing/CCPACSWingSparPositions.cpp
20.00% <0.00%> (-60.00%)
:arrow_down:

src/wing/CCPACSWingCell.cpp
21.27% <0.00%> (-56.87%)
:arrow_down:

... and 36 more

Merging #893 (https://github.com/DLR-SC/tigl/commit/37c6938a80606c785277871291712c9691ac1335) into master (https://github.com/DLR-SC/tigl/commit/e5d4ee8d0ab56b1b66729c2a9616e4fc7ea71472) will decrease coverage by 6.96%.

Ok, this is obviously wrong. 63.34% is the code coverage of the unit tests without integration tests...we need to cleanup CI some more and make sure that integration tests are also considered in pull requests. But that is something for another issue...


[Compiler] Handle lists in arguments

Generate Kotlin Lists for GraphQL lists arguments so that RealCacheKeyBuilder can traverse them and replace variables when needed.
For an exemple, the below query would not replace the tags_filter_1 and tags_filter_2 variables:
query getContent(
  $vendors: [Int!],
  $tags_filter_1: [Int!],
  $tags_filter_2: [Int!]
) {
  content(
    where:{
      state: {_eq: "available"},
      vendor_id: {_in: $vendors},
      _and : [
        { recipe_tags : { tag_id: {_in: $tags_filter_1}}},
        { recipe_tags : { tag_id: {_in: $tags_filter_2}}}
      ]
    }
  ) {id}
}

closes #2589

We need to carry this fix over to our dev-3.x branch.

We need to carry this fix over to our dev-3.x branch.

I'll merge once https://github.com/apollographql/apollo-android/pull/2604 is in

Released in 2.4.1 (also merged in dev-3.x as part of https://github.com/apollographql/apollo-android/pull/2625)


Address issue #122 according to Inf WG discussion

This also adds a definition of a Reproducible software component so that RDI does not become a venue for submissions that are not reproducible and can never be reproducible.

Thank you @DilipSequeira for the change. I have a question though - when available software can be private (currently no public availability details are mandated) is it okay to ask for RDI software binary to be downloadable from a public URL? I propose this condition to be added to available category as well.
Also if we enforce a reproducible software and available hardware, we can enable audit for such systems right?

Nvidia would like to see all software being downloadable from public URLs, but I don't think it's realistic to impose that on less forthcoming submitters. Changed it to "by anyone to whom the hardware is Available."

Thank you Dilip for the change. May be Nvidia can propose "Available to public (retail)" (both hardware and software) as a subcategory to "available" as it is clearly different from "available to OEMs".

I agree it would be good to refine the notion of Availability, but not in this PR. Maybe you could start an issue/PR for that, as a basis for working group discussion?

Sure Dilip. I can do that.

WG: Clarify rules around binary components. Otherwise, looks good.

@johntran-nv @erichan1 Can you review this in Training WG?

@DilipSequeira Can we do a change like below?
RDI systems using only Available hardware may use a Reproducible software stack, accompanied by instructions which would allow a reasonable expert user to download and install it on a hardware system similar to the submission system.
RDI systems may not be submitted as Available components  until the submission cycle after next or 221 days whichever is longer. This restriction is not applicable to RDI systems using available hardware and a Reproducible software stack

@arjunsuresh excellent suggestion. I've updated the PR accordingly.
@johntran-nv can training please re-review.

Thank you @DilipSequeira

A binary component is Reproducible if it is downloadable by anyone to whom the hardware is Available,
from a URL which must remain valid until a release of the software is Available.

I think we need to define what is meant by "a release" here. I imagine a vendor might want to include some optimisations into an engineering build of their driver in the run-up to a submission deadline. Expecting that same engineering build to eventually become Available would be unreasonable - as it may not have gone through a rigorous testing process. Rather, a proper release incorporating the same changes should serve as well.
Let's take a concrete example. Suppose that a submission deadline happens to be on the 1st of February, and the announcement is planned for the 28th of February. Suppose a submitter uses an engineering build u.v.w to obtain RDI results by the deadline. Suppose they make a proper release u.v on the 27th of February. Questions:

Does u.v.w need to be available at a URL disclosed to the Review Committee from the 1st to the 27th? (I assume, yes.)
Can the URL be invalidated when u.v is released? (I assume, yes.)
Would the result tables need to be updated to refer to u.v instead of u.v.w? (I assume, maybe.)
Most importantly, should the submitter guarantee that the performance/accuracy with u.v is the same as with u.v.w? (I assume, yes, but how?)

Fair point. What's meant here is "an Available release containing the optimizations" - which for practical purposes means, an available release whose performance on the benchmark is at least as good as the submitted results.
So I would say the answers to your questions are:
Yes, the URL must be disclosed
Yes, the URL can be invalidated on release of the software to customers
Ideally yes, because otherwise the submission becomes unreproducible once the URL is invalidated. I'm not sure we have a mechanism for this today.
Not quite - performance and accuracy must be at least as good as the submitted results, rather than the same.
I don't think it's any part of our goal here to remove the due diligence burden on submitters to ensure that the submission is reproducible in regard to performance and accuracy - just to give submitters the option to submit bleeding edge optimizations somehow.  The path that minimizes overall engineering work is to plan such that your software is at least in beta on submission day.

From a submitter point of view I think the proposal can be summarized as follows:
Suppose February 28 is the submission deadline and I have a new staging software on February 21 which I would like to use for submission (provided all runs go as expected). In this case I have the following options.

Try to make a beta release out of the staging build within a week (before February 28). In this case I can submit my results in the Available category.
Suppose my current build is not backward compatible or is very specific to a particular model. Then I have 2 options

Make the software binary available to download and then do a submission under RDI category. In this case, no 221 days rule is applicable.
Do not make the software binary available and do a submission under RDI category. In this case the 221 days rule is applicable.

The only issue I see here is for the last case. I'm not sure how MLCommons can ensure that the 221 days rule is followed as the software is completely closed (though hardware is available).

Training WG talked about it today. We generally think this is ok. Two questions.

How would the reproducible tag actually show up in the table? Just "RDI-reproducible" vs. "RDI"?
What does @TheKanter think? Since you think we shouldn't complicate the categories any more for results viewers.

RDI is explicitly understood and was designed to support results that are not reproducible already (e.g., enabling internal prototypes).  That was it's original intent. E.g., people submitting FPGA prototypes of hardware, internal-only products (e.g., Tesla systems were mentioned).
I am strongly opposed to any additional fragmentation of results. The results presentation is problematic as is already. Results need to be simpler, not more complicated.

I suggest we regard having a reproducible stack in RDI as a compliance issue (in the sense that you need to have compiled if you're going to submit in Available the next round), and we don't typically capture those in the results table.

@TheKanter @erichan1 If all the concerns are addressed can we please merge this PR? As the rule currently stands it is difficult to do any submission on an available hardware with not released software and in TinyML submissions too we are having issues with it.

I am a bit confused. If a submission is available, then every component
needs to be available how could you have an available submission that has
available hardware, but unavailable software?
On Wed, May 31, 2023 at 4:16 PM Arjun Suresh @.***>
wrote:

@TheKanter https://github.com/TheKanter @erichan1
https://github.com/erichan1 If all the concerns are addressed can we
please merge this PR? As the rule currently stands it is difficult to do
any submission on an available hardware with not released software and in
TinyML submissions too we are having issues with it.
—
Reply to this email directly, view it on GitHub
https://github.com/mlcommons/policies/pull/126#issuecomment-1571085407,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AJXLOK5SKWZZ4HSNBEXJTY3XI7GNLANCNFSM6AAAAAAQ2C747Y
.
You are receiving this because you were mentioned.Message ID:
@.***>

@TheKanter Exactly. So the proposal is to submit such results under RDI category. Since the hardware is available such submissions are exempt from the 221 days rule (if hardware is unavailable an RDI submission cannot be submitted under available category for 221 days).

@TheKanter This has been discussed in both Training and Inference WGs. Do you have any further open questions?

This PR was discussed in the recent chairs sync and prompted the need to review Preview and RDI rules. DavidT will be starting a proposal based on the discussion.


"ClassVer" entry is not mentioned

Some of the driver samples have an entry in their INF similar to the following:
[Version]
...
ClassVer = 4.0    ; (or some other number like 2.0)
...

I can't find any mention of this entry or how to set it in the documentation.
It shows up in many/all printer drivers, but I've seen it in the INF files for class=display and class=media drivers as well...

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

ID: 89e65b9c-b342-6df7-e878-4b88a9590363
Version Independent ID: 8f0d7eb7-cd13-2769-f20d-296ecc96d417
Content: INF Version Section - Windows drivers
Content Source: windows-driver-docs-pr/install/inf-version-section.md
Product: windows-hardware
Technology: windows-devices
GitHub Login: @tedhudek
Microsoft Alias: tedhudek

Good question @mr-intj.  Investigating

Thank you sir. All my Google-Fu has been able to tell me is that Printer INFs usually specify 4.0 and Display and Media sometimes specify 2.0. I'm assuming it's referring to the class driver installer version, but I can't seem to find any more info...

Added a brief blurb


When does error 414 occur?

Sorry in advance, my question is quite vague.
And it is quite vague because I cannot reproduce it correctly and found no place in cowboy code about it.
I have, sometimes, cowboy returning a 414 error. When that happens, I can simply resend the same query and it works. I don't expect one to solve this but could someone provide pointers as to where I can look in the code?
Thank you :)

https://github.com/ninenines/cowboy/blob/master/src/cowboy_protocol.erl#L156 I guess?
Increase the max_request_line_length option, it defaults to 4096 (that's included method, whitespace and HTTP version).

Thank you, should have closed this earlier, sorry.


NumberGuessingGame

NumberGuessingGame using html/css, and javascript
:warning: This is a Pull Request Template :warning:

Checkout all the things you've done by placing an x within the square braces below.

Proposed Changes

[] I've read the CODE OF CONDUCT and abide to it.
[] I've read the CONTRIBUTING.md
[] I've forked the repository.
[] I've created a branch and made my changes in it.
[] I understand opening a PULL REQUEST doesn't mean it will be merged for sure.

Added a numberGuessingGame


[$500] Chat - On clearing a draft message from a chat, LHN list reshuffles - Reported by: @parasharrajat

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

Action Performed:

Set  chat setting to "Most recent"
Select any chat that is not Pinned and has no draft from bottom of the LHN.
Add a draft message,.
Switch to another chat,
The Chat with draft should be moved top now.
Open that chat again.
Clear the draft.
LHN list reshuffles.

Expected Result:
It should reshuffle when you move to another chat same as when you add a draft.
Actual Result:
On clearing a draft message from a chat, LHN list reshuffles
Workaround:
Unknown
Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: 1.1.19-0
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/43995119/146195105-81063252-5a8a-474f-b0d9-5ddd810f0397.mp4
https://user-images.githubusercontent.com/43995119/146195118-dc13c0d9-d729-4f10-9fe8-1f4d4fce07e8.mp4
Expensify/Expensify Issue URL:
Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1639162750033700
Job post: https://www.upwork.com/jobs/~01ba23d541d89eeedd
View all open jobs on GitHub

Triggered auto assignment to @cead22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

Triggered auto assignment to @bfitzexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

Hey @mvtglobally, you tagged the wrong person on the issue. My handler is @parasharrajat . Also, there are many issues like this. I would suggest using only Slack names and no tagging until you know the handle.  E.g. my slack name is Rajat Parashar and no use of @. It would at least not alert the wrong person. May be prepare a list if that helps you. Another thing could be that we all can add our GH handle info in the Slack Profile description and then you can use that here.

@bfitzexpensify
The behavior was that if you add a draft to a chat that is currently active.

Pencil should be added but no reordering should happen on the LHN.
Now when you clear the draft, pencil should go away but not reordering should happen on the LHN.
List will only reorder in 1 or 2 when you move to another chat.

Point 1 and 2 was added recently in via two issues cc: @mountiny  and 1 seems to be work but not 2.

Cool, thanks for the explanation, I understand what you mean now and can reproduce. Figure this is likely External but will let @cead22 confirm that.

Priority mode has to be on Most Recent to reproduce

Triggered auto assignment to @parasharrajat (Exported)

Triggered auto assignment to @marcochavezf (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@parasharrajat can you apply for the reporting bonus? Thanks!

@bfitzexpensify I also added videos for a better representation

I will take this issue on as CME since I have worked on the implementation with @marktoman previously. Also I have assigned Kevin again as CM since I believe it was a bug that you got unassigned.

Thanks, @mountiny. Waiting for proposals before I post one :wink:

A few proposals in upwork, but still looking for an approved proposal to move forward.

Hmm, no proposals here, so not sure what they proposed with 😅

Reason:
To check whether a active report has a draft, following code is being used in shouldReorder function of SidebarLinks.js. If the active report has a draft, it returns false but if active report does not have a draft it returns true.
When length of newComment is zero, it is removed from the list of reportswithdraft. So, when the length of newComment becomes zero, sidebar reorders.
// Do not re-order if the active report has a draft and vice versa.
if (nextProps.currentlyViewedReportID) {
            const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false);
            return !hasActiveReportDraft;
        }

Proposal:
// Do not re-order if the active report has a draft and vice versa.

        const didActiveReportChange = currentlyViewedReportID !== nextProps.currentlyViewedReportID;

        if (nextProps.currentlyViewedReportID) {
            const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false);
            if (!didActiveReportChange) {
            return false;
            }
            return !hasActiveReportDraft;
        }

Here, if the active report does not change, then it returns false. The case of new messages is handled before this block of code. So, will not have impact on reordering in presence of new messages.

@sobitneupane Hi Sobit! I like your proposal, but unfortunately, after some investigation, I discovered it won't work as expected. See:
https://github.com/Expensify/App/blob/3c411711f9c641338694a85c0ebfd9a9ec97a9e2/src/pages/home/sidebar/SidebarLinks.js#L134-L136
On these lines we check if the active report changed. We return true if the active report changed. Therefore, we continue in execution of the shouldReorder function only if that is false. Int that case your proposed condition will always be true answer would always return false.
This will fix state problem, however, when you compose a message, draft is created and then when you send a message, the LHN should reorder. With this change, after sending the message, it does not get sent, because of this added statement.
The problem we have is that from current setup, it is hard to distinguish between draft being removed by deleting or sending.
Let me know if anything is not clear in my explanation! Thank you for your proposal and merry Christmas 🎄😊

Thank you for the response. I will try to dig deep.

Thanks for handling it @mountiny .

Still waiting for proposals 👀

Just updating the title to reflect that the price for this has been doubled.

Proposal
Previously I had proposed change in SidebarLinks.js shouldReorder function. But as @mountiny mentioned,

The problem we have is that from current setup, it is hard to distinguish between draft being removed by deleting or sending.

So, main idea behind my new proposal is to set REPORTS_WITH_DRAFT false only when either message is sent or component unmounts with no draft. Thus, even when draft is cleared, REPORTS_WITH_DRAFT will be true until the component unmounts.
My proposal is to

Remove
https://github.com/Expensify/App/blob/ca085ae41de731893bf6c99cfd3401533ef1196e/src/pages/home/report/ReportActionCompose.js#L345-L347
Add

if(this.state.isCommentEmpty) {
            Report.setReportWithDraft(this.props.reportID.toString(), false);
        }

in componentWillUnmount()
https://github.com/Expensify/App/blob/ca085ae41de731893bf6c99cfd3401533ef1196e/src/pages/home/report/ReportActionCompose.js#L198-L205
3.Add
Report.setReportWithDraft(this.props.reportID.toString(), false);

in submitForm() after this.updateComment('');
https://github.com/Expensify/App/blob/ca085ae41de731893bf6c99cfd3401533ef1196e/src/pages/home/report/ReportActionCompose.js#L457-L474

@sobitneupane Thank you for this proposal. I see the idea behind this, however, it seems still quite complicated and a workaround solution. This functionality has worked previously so I would certainly try to look at what has broken it.

Waiting for proposals still.

proposal
on shouldReorder in SidebarLinks.js,
https://github.com/Expensify/App/blob/ddae43334629449e1cd4a9296fa18b234d7ccd94/src/pages/home/sidebar/SidebarLinks.js#L147-L153
Shouldn't we return false at the end? and the code above it could be removed.
-        // Do not re-order if the active report has a draft and vice versa.
-        if (nextProps.currentlyViewedReportID) {
-            const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false);
-            return !hasActiveReportDraft;
        }

-        return true;
+       return false;

Looking into this!

The core problem is to identify a way how to distinguish whether the current report deleted draft because of:

message sent - should reorder
message deleted - should not reorder

Because if we weill have a way to do it, we can then decide.

How about keeping track of currently viewed report maxSequenceNumber. If it change, then shouldReorder return true.

@K4tsuki That could work I guess. Would you propose full solution (with testing it actually works) or should I look into using it?

@mountiny I will try to test it and make the proposal.

New proposal:
I prefer to return false at the of shouldOrder, but some things need to be fixed, because returning false prevent reorder for actions like: user pin/unpin a report, new IOUstanding report coming, new message coming and user send a message. So I need new state:
reportsStats: {
     pinnedCount: 0,
     hasOutstandingIOUCount: 0,
     totalReportsActionCount: 0,
}

And then need to modify shouldReorder, removing some parts of it and return false at the end. This could be be :
  shouldReorder
    static shouldReorder(nextProps, orderedReports, currentlyViewedReportID, unreadReports) {
        // We do not want to re-order reports in the LHN if the only change is the draft comment in the
        // current report.

        // We don't need to limit draft comment flashing for small screen widths as LHN is not visible.
        if (nextProps.isSmallScreenWidth) {
            return true;
        }

        // Always update if LHN is empty.
        if (orderedReports.length === 0) {
            return true;
        }

        const didActiveReportChange = currentlyViewedReportID !== nextProps.currentlyViewedReportID;

        // Always re-order the list whenever the active report is changed
        if (didActiveReportChange) {
            return true;
        }

-        // If any reports have new unread messages, re-order the list
-        const nextUnreadReports = SidebarLinks.getUnreadReports(nextProps.reports || {});
-        const hasNewUnreadReports = nextUnreadReports.length > 0
-            && _.some(nextUnreadReports,
-                nextUnreadReport => !_.some(unreadReports, unreadReport => unreadReport.reportID === nextUnreadReport.reportID));
-        if (hasNewUnreadReports) {
-            return true;
-        }

-        // Do not re-order if the active report has a draft and vice versa.
-        if (nextProps.currentlyViewedReportID) {
-            const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false);
-            return !hasActiveReportDraft;
-        }

-        return true;
+       return false;
}

To compute the reports statistics/counts the function could be.:
static getReportsStatistic(reportsObject) {
        const reports = _.values(reportsObject);
        if (reports.length === 0) {
            return [];
        }

        let pinnedCount = 0, hasOutstandingIOUCount = 0, totalReportsActionCount = 0;
        _.each(reports, (report) => {
            if (report.isPinned) {pinnedCount += 1;}
            if (report.hasOutstandingIOU) {hasOutstandingIOUCount += 1;}
            totalReportsActionCount += report.maxSequenceNumber;

        })

        return ({ pinnedCount, hasOutstandingIOUCount, totalReportsActionCount});
    }

Here I use totalReportsActionCount, because if user send a message or new message is coming then LHN should reorder.
In getDerivedStateFromProps, add const reportsStats = SidebarLinks.getReportsStatistic(nextProps.reports); as a new reportsStats state.
And then modify check in:
https://github.com/Expensify/App/blob/0b58649585f5a44682e9e5904b8c53ac8b496745/src/pages/home/sidebar/SidebarLinks.js#L168
to be : const orderedReports = shouldReorder || !_.isEqual(repportStats, prevState.reportsState)
I also need to add Report.setReportWithDraft(this.props.reportID.toString(), false); before:
https://github.com/Expensify/App/blob/0b58649585f5a44682e9e5904b8c53ac8b496745/src/pages/home/report/ReportActionCompose.js#L388
to modify currently active report_wtih_draft onyx data, because  it is late to modify the data after submit. If it is late report will be placed at draft reports region after user send a message.
  Result:
https://user-images.githubusercontent.com/1313124/153349115-2a32c5d5-4668-43ec-9ecb-9f869127535e.mp4

This could solve another bug (I think a bug) about LHN with new standing IOUAmount coming  while user edit compose box in other room, the new standing report won't pass draft region reports.
 new standing IOU,
Current:

After:

@K4tsuki Thank you for such a thorough proposal. Overall it looks very nice, my only concern is yet another static function added, where we would go over all the reports and their actions, which for accounts with 100+ reports might be unnecessary load to do before each reshuffling.
Are you sure we cannot use the maxSequenceNumber or some other already existent and computed variable to determine if things have changed?
cc @parasharrajat for review and thoughts.

@mountiny current source code also iterate through all report by using underscore.js functions.
in my proposal I am using basic iteration function _.each it is faster than current source code that depend on below check which is also iterate through  all reports by using undersore functions:
https://github.com/Expensify/App/blob/bdf3a7483ced85759d2a4bbc248b82dd62f36288/src/pages/home/sidebar/SidebarLinks.js#L139-L142
https://github.com/Expensify/App/blob/bdf3a7483ced85759d2a4bbc248b82dd62f36288/src/pages/home/sidebar/SidebarLinks.js#L108
I  am using totalActionCount that could replace this check, iterate only done once on all reports.
I benchmarking 10000 times operation of  these two code ( comparing my proposal with current code):

do report stats is my proposal and next unread is current code. As you can see, my code is faster than current code, half faster or more.

More over my proposal don't need to call getRecentReports (it is heavy operation, I explain it below) unnecessarily, it will only call it  if shouldReorder is true.
In my proposal, shouldReorder almost always returning false. This is important because SidebarLink is rendered many times. For example when user send a message, SidebarLink is rendered many times but it only need to call getRecentReports once, hence my proposal filtering that unnecessary call to getRecentReports.  Current source code will almost always return true and always call getRecentReports even if there is no reshuffle needed.
In my code:
 const orderedReports = shouldReorder || !_.isEqual(repportStats, prevState.reportsState)`
            ? SidebarLinks.getRecentReports(nextProps)
            : prevState.orderedReports

Here we don't call getRecentReports unnecessarily. getRecentReports is callingl getSidebarOptions which is calling getOptions which is heavy operation:
https://github.com/Expensify/App/blob/bdf3a7483ced85759d2a4bbc248b82dd62f36288/src/libs/OptionsListUtils.js#L351
Here it will iterate and process all reports data using many underscore functions.

So based on my data above,  I believe my proposal is more faster and lighter than current implementation.

Thanks, @mountiny for taking care of it. I am currently on a tight schedule. I will try to review this asap.

@parasharrajat No worries, I don't think there is a high urgency to this!

@parasharrajat I know you are busy, but any timeframe for this review? You can let me know if you cant review this one, it is fine, very understandable.

I will now. I will have to add some kind of webhook for reminders... things get lost until

Haha, Melvinbot is here for that.

Ok. I looked at the proposal. I agree it is far better than the previous one but only if it works. @mountiny Could you please help me with the list of changes when the sidebar link should not update? I can't quite remind that. You have been involved in this change since the start so maybe you have it handy.
Questions: @K4tsuki

When a new message arrives, the report becomes unread. It should be marked bold. Then if you are not calling SidebarLinks.getRecentReports(nextProps). How will the data be updated in the sidebar?
I am unsure of where are you tracking the daft messages for the report in your stats.

Of course!
I think the order is:

Pinned
IOUs
Unread messages
Draft (if created and report was changed)
Other messages

Reorder:

when you pin/unpin
when a new message arrives/ when you get IOU
when you send a message/create IOU
when you change reports

this is when the draft in the current message should show up in LHN

Do not reorder:

when you start typing and you create a draft, do not reshuffle LHN
the most important one - when you have a draft (after you created a draft, changed reports and the draft chat has moved up with the pencil icon) and you delete it, do not reshuffle until you change reports in LHN

Let me know if there is anything else left

That should be it. Thanks.

When a new message arrives, the report becomes unread. It should be marked bold. Then if you are not calling SidebarLinks.getRecentReports(nextProps). How will the data be updated in the sidebar?

For unread message it still can be updated, because I am tracking all maxSequenceNumber in getReportsStatistic. But for other data for example report name, personal details, icons, etc it is not being updated correctly. So I am back to returning true at the end of shouldReorder function and keep calling on SidebarLinks.getRecentReports(nextProps) by default.
  getReportsStatistic
static getReportsStatistic(reportsObject) {
        const reports = _.values(reportsObject);
        if (reports.length === 0) {
            return [];
        }

        let pinnedCount = 0, hasOutstandingIOUCount = 0, totalReportsActionCount = 0;
        _.each(reports, (report) => {
            if (report.isPinned) {pinnedCount += 1;}
            if (report.hasOutstandingIOU) {hasOutstandingIOUCount += 1;}
            totalReportsActionCount += report.maxSequenceNumber;

        })

        return ({ pinnedCount, hasOutstandingIOUCount, totalReportsActionCount});
    }

I am unsure of where are you tracking the daft messages for the report in your stats.

If I am returning false the at the of shouldReorder, tracking for draft message isn't needed. Because it default to not reorder the LHN.
But  I am back to returning true add the end, here is the tracking of the draft.

Add draftActive variable above Sidebarlinks declaration.
Here diff in shouldReorder function:
// Always re-order the list whenever the active report is changed
if (didActiveReportChange) {
+  draftActive = false;
  return true;
}

+const reportsStats = SidebarLinks.getReportsStatistic(nextProps.reports);
+if (!_.isEqual(reportsStats, prevReportsStats)) {
+  draftActive = false;
+  return true;
+}

-// If any reports have new unread messages, re-order the list
-const nextUnreadReports = SidebarLinks.getUnreadReports(nextProps.reports || {});
-const hasNewUnreadReports = nextUnreadReports.length > 0 && _.some(nextUnreadReports, (nextUnreadReport) => -!_.some(unreadReports, (unreadReport) => unreadReport.reportID === nextUnreadReport.reportID));
-if (hasNewUnreadReports) {
-  return true;
-}

// Do not re-order if the active report has a draft and vice versa.
if (nextProps.currentlyViewedReportID) {
  const hasActiveReportDraft = lodashGet(nextProps.reportsWithDraft,`${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`,false);
+  if (hasActiveReportDraft) {
+    draftActive = true;
+    return false;
+  }
+  if (draftActive) {
+    return false;
+  }
-  return !hasActiveReportDraft;
}
return true;

Because flow of draftActive value is important, reportWithDraft  of currently active report need to be updated first before submit new comment , I need Promise resolve in ReportActionCompose  in submitForm:
new Promise((resolve, reject) => {
  Report.setReportWithDraft(this.props.reportID.toString(), false);
  resolve();
}).then(() => {
  this.props.onSubmit(trimmedComment);
});

What is draftActive variable? I think you are tracking the report draft status but this. Something; like when user add/remove the draft you use this variable. Right?
If so, this is a bad approach as the sidebar is not related to Report that is currently open. You are trying to mix states between components. This is considered a bad pattern. On the other hand, the stats technique is fine.

Because flow of draftActive value is important, reportWithDraft of currently active report need to be updated first before submit new comment , I need Promise resolve in ReportActionCompose in submitForm:

I am not sure of the side effects of this change. Introducing async code in the sync code may cause other issues.
To me, these changes feel like adding patches to the code to overcome the problem instead of a proper solution to solve the problem.
Sidenote: I see that the original proposal is changed thrice instead of incremental updates to the original proposal. It indicates that proposal needs more work. I would suggest fully testing your proposal/approach before proposing one. It saves time for both parties. Ask more questions before you propose.

What is draftActive variable? I think you are tracking the report draft status but this. Something; like when user add/remove the draft you use this variable. Right?

I am only set the draftActive to true inside if (nextProps.currentlyViewedReportID) { so it is tied to currently viewed report.  Then reset it to false if activeReport change or report stats change.
For clean code purpose, draftActive could be stored in state.

I am not sure of the side effects of this change. Introducing async code in the sync code may cause other issues.

For promise and resolve part it is because of when user  send a comment, reportWithDraft of currently active report, the reportWithDraft  is still saved as true even if I put the Report.setReportWithDraft(this.props.reportID.toString(), false); before this.props.onSubmit(trimmedComment); .  Every onyx action is async and Report.setReportWithDraft(this.props.reportID.toString(), false); is late. I need these two code to run in the right order so I use  promise resolve.
If I don't do this after user send a message, the row is reshuffled to draft region because Report.setReportWithDraft(this.props.reportID.toString(), false);  is late.

To me, these changes feel like adding patches to the code to overcome the problem instead of a proper solution to solve the problem.

Yes, if there is no promise resolve and no change in ReportActionCompose I think that is better.

Sidenote: I see that the original proposal is changed thrice instead of incremental updates to the original proposal. It indicates that proposal needs more work. I would suggest fully testing your proposal/approach before proposing one. It saves time for both parties. Ask more questions before you propose.

I am sorry.

@parasharrajat surely you are very busy, will you be able to follow the discussion here?

I don't think there is something on the plate for discussion. I indicated a few things that need to be improved. I saw the explanations but I still don't see improvements to the proposal. So I am waiting on that.

To me, these changes feel like adding patches to the code to overcome the problem instead of a proper solution to solve the problem.

This statement indicates the review of the existing proposal.

if there are no other ways to solve it I would give it another look.

Here I asked for improvements.
If I was not clear enough, I think this can be improved in the following way :-

Anything that is happening with the active report should either be moved to ReprotScreen or Onyx or something else. But not in the sidebar.
How can we remove the async (promise.resolve) code out of the sync logic. What is the best way to refactor?

Thanks for the update.
@kevinksullivan could we please double the price on this issue? Thank you very much!

@kevinksullivan Could we double the price here again, please? 😊

Proposal
My proposal extends the currently implemented idea, which is based on the original idea where this scenario was working. (The original implementation had issues caused by frozen rendering, hence the change.) Both of them compare future and previous states.
We are already caching props that we need to compare with a next state, now we just extend it.

Add reportsWithDraft (constructor):

  this.state = {
    reportsWithDraft: props.reportsWithDraft,
    ...

Add a new parameter to shouldReorder and pass it to it (getDerivedStateFromProps):

const shouldReorder = SidebarLinks.shouldReorder(..., prevState.reportsWithDraft, ...);

Now we detect changes for drafts and prevent re-order if any. Add this inside shouldReorder (below the hasNewUnreadReports-related code):

if (nextProps.currentlyViewedReportID) {
  if (lodashGet(reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${nextProps.currentlyViewedReportID}`, false)) {
    return false;
  }
}
...

Alternatively, we can prevent re-order if any draft was removed (not only the active one), but if I understand that is not the requirement:
const reportsWithDraftOnly = _.filter(_.keys(reportsWithDraft), key => reportsWithDraft[key]);
const nextReportsWithDraftOnly = _.filter(_.keys(nextProps.reportsWithDraft), key => nextProps.reportsWithDraft[key]);

if (reportsWithDraftOnly.length > nextReportsWithDraftOnly.length) {
  return false;
}
...

Update the value (getDerivedStateFromProps):

return {
    reportsWithDraft: nextProps.reportsWithDraft,
    ...

@marktoman 's proposal looks convincing.

Alternatively, we can prevent re-order if any draft was removed (not only the active one), but if I understand that is not the requirement:

Good point. @mountiny What do you say?
cc: @mountiny
:ribbon: :eyes: :ribbon:  C+ reviewed

📣 @marktoman You have been assigned to this job by @mountiny!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻
Keep in mind: Code of Conduct | Contributing 📖

@marktoman Good to see you back!
@parasharrajat thank you for reviewing!

Alternatively, we can prevent re-order if any draft was removed (not only the active one), but if I understand that is not the requirement:

I am not sure if there would be an intended scenario when a draft would get cleared on a non-active report so we can skip this (or have I misunderstood what this is means?)

Thanks, I've applied. Agree that this is probably not a realistic use case.

@marktoman Bumping here, it has been a while now, what is your ETA for the PR? Thank you 🙇

@mountiny  While the code is ready, I need to test it thoroughly and specify it as testing steps. I will take it as a priority this week.

@marktoman How does it look like Mark? Any problems we can help with? cc @parasharrajat

@marktoman paid. @parasharrajat just hired for reporting and c+ pay out. Once you accept I'll take care of payment.

Accepted @kevinksullivan

All set,  thanks @parasharrajat .


issues #2 #3 #4

Please review and tell me what you think

to address #2 #3 #4

These are good changes and I'll merge it if you can remove the .idea folder from your PR. While for a single person adding this folder makes sense, it interferes with file locations (specifically gradle home) whenever anyone else tries to load the project. It's all studio generated setup for the local install.

These are good changes and I'll merge it if you can remove the .idea folder from your PR. While for a single person adding this folder makes sense, it interferes with file locations (specifically gradle home) whenever anyone else tries to load the project. It's all studio generated setup for the local install.

Yeah I see your put - although this folder just contains relative paths rather then absolute ones.
https://intellij-support.jetbrains.com/hc/en-us/articles/206544839-How-to-manage-projects-under-Version-Control-Systems
I didn't test my changes too much so please feel free to test again :)
Thanks for the feedback!


Demo Project files by Oluwafemi

A simple implementation for the Demo calculator in C
Available operations include Addition, Subtraction, Division and Multiplication

Alright, will do

Executable removed, files ready to be merged


Add fix for Wayland support in GLFW.

Please check if the PR fulfills these requirements

[x] The commit message follows our guidelines
[ ] Tests for the changes have been added (for bug fixes / features)
[ ] Documentation has been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
This change enables support for the Wayland windowing system inside our build of GLFW.
Is there an open issue that this resolves? If so, please link it here.
No issue to track for this bug.
What is the current behavior? (You can also link to an open issue here)
GLFW will compile for X11 only, causing wayland-only systems to fail.
What is the new behavior (if this is a feature change)?
Wayland-only systems should be able to use the GLFW windowing backend correctly.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No.
Other information:
Documentation that directed this change can be found here: https://www.glfw.org/docs/latest/compile.html#compile_options_wayland

Nevermind. Ignore. Just read that you need the X11 interface for better NVIDIA support anyway.

We should probably keep this option on anyway, as most applications will gracefully fall back to X11 if Wayland is not present. It's unlikely that a NVIDIA user would be using Wayland at the current time anyway.

That would leave this blocked, as GLFW has a bug (fixed in main but not released yet) that actually prevents X11 & Wayland support from compiling together (as we accidentally discovered today 😅)

Oh, that's unfortunate. In that case, yeah, we should open a blocked issue for Wayland support.


buildHealth fails when using Fragment Testing

Build scan link
Not required for this problem.
Plugin version
0.71.0
Gradle version
6.8.3
Android Gradle Plugin (AGP) version
4.2.0-beta06
Describe the bug
To use fragment testing artefact, the official documentation recommend us to use: debugImplementation "androidx.fragment:fragment-testing:$fragment_version". However, when running buildHealth we fail with the following error message: check Additional context.
The error is right as the dependency can not be found on release variant for unit tests and buildHealth is trying (I'm guessing) to verify the release variant.
To Reproduce
Steps to reproduce the behavior:

Include fragment-testing artefact as recommended in the docs (debugImplementation);
Create any test that uses any class from fragment-testing;
Try to execute buildHealth.

Expected behavior
It should be possible to use both fragment-testing and buildHealth in a same project.
One solution I see is to be able to filter a specific build variant (e.g., ignore "unit test release" but still run for all the rest).
Additional context
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (4, 30): Unresolved reference: testing
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (16, 9): Unresolved reference: launchFragmentInContainer
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (17, 13): Unresolved reference: onFragment
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (18, 35): Function '<ERROR FUNCTION>' should return Unit to be used by corresponding operator '+='
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (19, 45): Function '<ERROR FUNCTION>' should return Unit to be used by corresponding operator '+='
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (21, 13): Unresolved reference: recreate
e: /Users/marcellogalhardo/Developer/OpenSource/retained/fragment/src/test/java/dev/marcellogalhardo/retained/fragment/FragmentRetainedObjectTest.kt: (22, 13): Unresolved reference: onFragment

Could you provide a minimal reproducer? That would make it easier for me to find the root problem and resolve it.

It would also be useful if you could provide a link to the docs you are referring to. (As well as a reproducer.)

Hey @autonomousapps,
Sorry for the late reply, I miss the notification.
You can find the docs about Fragment Testing here: https://developer.android.com/guide/fragments/test
Simple add the dependency should cause the problem (as you'll see it is added only to debugImplementation):
dependencies {
    def fragment_version = "1.3.3"

    debugImplementation "androidx.fragment:fragment-testing:$fragment_version"
}

I will create an example and send you in the next couple of hours.
Sorry again for the delay, and thank you for the support! 🙏

Hey @autonomousapps,
sorry for the late reply, I missed your comment before.
You can find a sample here, simple clone and run ./gradlew :buildHealth. I added details in the readme, including the error output and the classes that caused the error in this sample project.
The documentation for fragment testing can be found here.
Thank you in advance for the support!

Hey @autonomousapps - I'm just checking out if my sample project works as expected and if I can help with any extra information. Thank you!

Hey, thanks for the nudge. I'll take a look!

@marcellogalhardo I cloned your project and reproduced the error. I also reproduced the error by simply running ./gradlew app:testDebugUnitTest -- your code doesn't compile. Note that your test code is in the src/test directory, and your only test dependency is JUnit4. The compiler is complaining because it can't find the other symbols. Looks like you've only declared them in androidTestImplementation.

Hey @autonomousapps - I'm so sorry for not simulating it properly previously. I updated the sample project and now you should be able to simulate the problem. Below you find the output of both commands.

./gradlew app:testDebugUnitTest output:

./gradlew buildHealth output:

Thank you a lot and I'm sorry again for not simulating it properly before.

Thanks for the update. I see the issue now. It's still down to the fact that your code doesn't compile under certain circumstances. When you run buildHealth, this is the task that fails:
> Task :app:compileReleaseUnitTestKotlin FAILED

In other words, if you tried to run release unit tests, rather than debug (app:testReleaseUnitTest), that would fail. This is because you've only added your dependency to debugImplementation. The probably would disappear if you used implementation, or even testImplementation. It's unclear to me why Google is saying to add that dependency to debugImplementation rather than a test configuration, since this seems to be a test-related artifact.
You could also resolve this by disabling release unit tests, which is a feature AGP provides.

Hey @autonomousapps - thank you for the update.
From my limited understand, Google recommends you to add as debugImplementation due to the EmptyActivity the fragment-testing uses internally: if we added as an implementation it won't be merged to the manifest and the test should fail.
Oh, the idea of disabling release unit tests sounds good for me (as I don't really use them). For those who find this issue with the same problem, you can disable release unit tests as following:
android {
    testOptions {
        unitTests.all { task ->
            // Verify if task should be enabled.
            task.isEnabled = task.name != "testDebugUnitTest"
        }
    }
}

I will close the issue as the solutions provided are satisfactory for my problem. Thank you for all the support @autonomousapps  and for the great library. 😄


[CELEBORN-1791] All NettyMemoryMetrics should register to source

What changes were proposed in this pull request?
StorageManager and ReadBufferDispacther does not register netty metrics.
Why are the changes needed?
All NettyMemoryMetrics should register to source
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing UTs.

ping @RexXiong Pls take a look