recipes with stacks  1.12.2

Is there a posibility to use craft recipes with stacks or more than one item?
I did the folowing recipe:

But can be crafted with one of each item

That is an issue of spigot's crafting system. We may add a workaround later but it is works fine now.


Update Dockerfile

Updated to 4.2.4
This hotfix resolves a critical regression with ruTorrent v4.2.3 when adding a new torrent. It's crucial to upgrade.

Updated to 4.2.4 This hotfix resolves a critical regression with ruTorrent v4.2.3 when adding a new torrent. It's crucial to upgrade.

Thanks a lot. A crucial update from version 4.2.3 (all changelog 4.2.2...4.2.4)

ruTorrent v4.2.5 is the most stable release. It's safe to merge this commit.
Perhaps change title to "Upgrade to ruTorrent v4.2.5 and squash your commits.

I think i did it right. Sorry, I had these changes locally and didn't push it. I Think it's done right now though.

Ah gotcha. Did it:) thanks for that @V33m !


Mapping OpenApiGenerator generated classes

I'm using the OpenAPI Generator for generating the REST api from the API specification.
Unfortunately, it generates members inside the data classes with fully specified packges e.g. val firstname: kotlin.String? = null.
Trying to map these kind of generated classes leads to the following error:
[ksp] java.lang.IllegalStateException: Do not know how to convert kotlin.String with name firstname and path firstname
generated here:
https://github.com/s0nicyouth/kmapper/blob/bb67e21358533556214dfaaf13f546e14a0c0023/processor/src/main/kotlin/com/syouth/kmapper/processor/convertors/DataClassTypeConverter.kt#L57
I assume the root cause is the fully specified name including the package. I haven't been able to verify this, though. If so, I wonder how this could be handled.
Examples of two generated classes:
package org.example.api.model

import java.util.Objects
import com.fasterxml.jackson.annotation.JsonProperty
import jakarta.validation.Valid
import jakarta.validation.constraints.*
import io.swagger.v3.oas.annotations.media.Schema

/**
 *
 * @param id unique id
 * @param firstname firstname
 * @param address
 * @param createdDate
 */
data class User(

    @Schema(example = "null", readOnly = true, description = "unique id")
    @field:JsonProperty("id") val id: java.util.UUID? = null,

    @Schema(example = "null", description = "firstname")
    @field:JsonProperty("firstname") val firstname: kotlin.String? = null,

    @field:Valid
    @Schema(example = "null", description = "")
    @field:JsonProperty("address") val address: kotlin.collections.List<Address>? = null,

    @Schema(example = "null", description = "")
    @field:JsonProperty("createdDate") val createdDate: java.time.OffsetDateTime? = null
) {

}

import java.util.Objects
import com.fasterxml.jackson.annotation.JsonProperty
import jakarta.validation.Valid
import jakarta.validation.constraints.*
import io.swagger.v3.oas.annotations.media.Schema

/**
 *
 * @param id unique id
 * @param town location
 */
data class Address(

    @Schema(example = "null", readOnly = true, description = "unique id")
    @field:JsonProperty("id") val id: java.util.UUID? = null,

    @Schema(example = "null", description = "location")
    @field:JsonProperty("town") val town: kotlin.String? = null
) {

}

data class DomainUser(val id: UUID, val firstname: String)

@Mapper
interface DomainUser2UserMapper {
    fun map(du: DomainUser) : User
}

Oh, I'm sorry.
It seems that I've been following a red herring :man_facepalming:
The member definitions differ between to and from regarding nullability.
 val firstname: kotlin.String? = null vs val firstname: String
I haven't worked out how to code the mapper, yet, in case the different definition was on purpose, though

I might have found some solution, even though, I'm not sure, if a need for this conversion arises, it wouldn't indicate a flaw in the applications design.
The solution below delegates/leaks domain knowledge to the mapper.
Anyway, it does work. However, I'm curious if there was a different solution.
internal interface UsersMapperSupport {
//    fun mapUUIDToNullableUUID(i: UUID): UUID? = i
    fun mapNullableUUIDToUUID(i: UUID?): UUID = i ?: UUID.randomUUID()
    fun mapNullableStringToString(i: String?): String = i ?: ""
}

@Mapper
internal interface DomainUser2ApiUserMapper : UsersMapperSupport {
    fun map(du: DomainUser): User
}

@Mapper
internal interface ApiUser2DomainUserMapper : UsersMapperSupport {
    fun map(au: User): DomainUser
}

I'd argue that safer route would be to properly mark required fields in open api schema :). This workaround with mapping nullable string to string or empty might bite you later on

That's an interesting question actually. It is possible to map nulls to non-nulls actually something like that:
nonnullable = nullable ?: throw IllegalStateException("Value is null") and for user converters it works like that already, so probably I will implement it that way.

@s0nicyouth but would be that optional? In my opinion it is beneficial to have compile-time error when types mismatch this way

@jakoss Yes, indeed, making it optional might be a good idea.


Napp management refactor

This refactor changes the location of the napps in a way that now we have a "installed" napps dir and the "enabled" napps dir will contain a symbolic link to the installed napp.

Closed in favor of #153


Jquery ".on" doesn't work with emojionearea

Does anyone know how do I define
var $inputMessage
in the example below to work with emojionearea? It's working fine when I disable emojionearea, but with emojionearea nothing.
var $inputMessage = $('#msginput');

$inputMessage.on('input', function() {
updateTyping();
});

Have you tried reading README?
ref https://github.com/mervick/emojionearea#methods
ref https://github.com/mervick/emojionearea#keyup
var $inputMessage = $('#msginput');

$inputMessage.emojioneArea({
  events: {
    keyup: function() {
      console.log(this.getText());
      // updateTyping();
    }
  }
});

Example: https://jsfiddle.net/mervick/audv6xpb/1/

Thank you for your reply.
Can you deomonstrate it with my case, how can I call function updateTyping()?
$inputMessage.on('input', function() {
updateTyping();
});

$inputMessage.emojioneArea({
  events: {
    keyup: function() {
      updateTyping();
    }
  }
});

Unfortunately, it does not work

Yes, it works

It does not work.
function updateTyping () {
if (!typing) {
typing = true;
socket.emit('typing');
}
lastTypingTime = (new Date()).getTime();

setTimeout(function () {
var typingTimer = (new Date()).getTime();
var timeDiff = typingTimer - lastTypingTime;
if (timeDiff >= TYPING_TIMER_LENGTH && typing) {
socket.emit('stop typing');
typing = false;
}
}, TYPING_TIMER_LENGTH);

}

And could you tell why emojionearea loads so slow, especially in mobile phones.
This version:
https://mervick.github.io/emojionearea/
works fine
but
this
https://github.com/mervick/emojionearea
is really slow. Takes some 2 seconds to appear emiji chooser in text area.

Yes, it works, thanks and sorry.

Emojioneare works
example https://jsfiddle.net/mervick/audv6xpb/4/


patchShebangs does not take into account NixOS specific setup

patchShebangs happily rewrites /usr/bin/pkexec to some /nix/store/ path. I think this is very undesired behaviour and the right behaviour would be to take into consideration that store paths like /nix/store/.../pkexec are never valid interpreters (and thus the rewrite should not happen or it should be rewritten to the system version (/var/...)).

I'm not sure that's a problem of patchShebangs itself: it just takes whatever is first on the current $PATH (when running patchShebangs; it uses command -v foo). I think that's mostly the right thing to do.™
Note that e.g. in the case when we patch scripts to be runnable during the build (only), we don't even have access to /var or /run, so we don't want to use such paths (in that case). I see no simple drop-in improvement of the semantics ATM. Perhaps the packages containing pkexec and similar should contain hooks to override those particular cases.

@wwat: I don't know the reason for zoneminder to use pkexec shebang; I patched it so that it only uses pkexec on the command that really needs privileges (the shellout to systemd), instead of running that whole perl script via pkexec. We should just push that patch upstream and the issue is gone.
I agree with @vcunat, there is not an obvious way to improve the situation in the generic builder.

@bjornfor Oh, good find. I thought there where calls coming from a lot of places.
@vcunat I saw an IRC message saying something along the lines that patchShebangs is not well documented, either. (I also had to read the code at some point (but that's not how it's supposed to work)).
There are also various places where assignments of the following form are being done:
a="$1"

The quotes are not needed in that case.
It should be very easy to find all the places where this happens via a simple grep.

@wwat: I might be wrong, but so far I haven't had any more pkexec issues after that patch. Quoting shell variables is just good style. (What would happen if someone did pass something with whitespace?)

@bjornfor It is not needed. It only tells me that whoever has written the NixOS build scripts wasn't very good at it. Read the POSIX standard on sh and you will see that I am right.
Or just run this in your shell:
a='1 b c '
b=$a
echo "$b"

@wwat: Oh, I didn't know about that. It's probably me that added quotes to variables in patchShebangs (I touched it once).
I've seen many broken shell scripts caused by to little quoting, so I'm a bit reluctant to dropping the quotes, even in places where it's safe to do so. (My personal rule is to quote everything that's not explicitly meant for word splitting.)

I think these extra quotes are a completely unimportant issue (and off-topic anyway).

patchShebangs writes the path to the command that is first found in $PATH. I think it should be the responsibility of the user (aka. developer writing nix package expression) to ensure that $PATH is setup correctly.
Even if patchShebangs were to hardcode some paths in /var/setuid-wrappers/ (where that makes sense), there will probably be cases where that behaviour is wrong. The current behaviour is just a healthy separation of mechanism and policy, IMHO.
Closing.


Set icon on every node of the tree widget [WIP]

Example code: https://gist.github.com/Dayof/37363d46c08ea2b36794b6ad2b88a54a

@freakboy3742 I'm working on this api yet, what do you think if we set a icon on a specific node of the tree passing its id, like this -> self.tree.set_icon('icon.png', self.root_one_id)? And if the user want set the icon for all the nodes of the tree just don't pass any id, like this -> self.tree.set_icon('icon.png').

I can see what you're driving at. However, my immediate thought was that the icon is a property of the node, not the tree. The tree is a visualisation of a data structure; that data structure should have some sort of interface - for example, how do I change the label of a particular tree node? Or collapse a particular tree node? This could be done with an API on the tree itself, but that feels a bit like "action at a distance" to me.


Restore Wizard Needs Horizontal Scrollbar when Resizing Columns

Try to resize the columns in the Restore plan to be very large, like so, and notice that no scrollbar appears, so data from every other column is inaccessible:

@yualan I believe this has always been like this, please triage.

@chlafreniere looks like this is fixed in latest bits... Can you please let me know if you still see this issue?

Please reactivate if you still see this


override Login forbidden

Hi,
Can anyone help me to override the default authentication error message "Login forbidden" to "Incorrect Login" .
Thanks,
hare

I experimented this same error using semantic:accounts-ui. I just remove the pack and greats news... it work...

Still have the same issue with semantic-ui package

which error/issue?
This was about changing the appearing text... wasn't it?


Add cover page and selection options for media

Good catch. That's what I get for copy/paste

@rslibed  ok updates made


scrollIntoView can fail on android due to View collapsing

See https://github.com/facebook/react-native/issues/3282#issuecomment-201934117
the newly added _measureElement method can actually return nothing if view collapse on Android.
In such case I think we should warn the user and recommend him to use collapsed={false} to ensure measures can be taken.
cc @sebasgarcep

@slorber For some reason I never saw this issue in my notifications. Did you solve this issue?

Hi,
I actually use https://github.com/slorber/react-native-scroll-into-view and the api design prevent this issue:
https://github.com/slorber/react-native-scroll-into-view/blob/master/scrollIntoView.js#L409

@slorber Could you send a pull request with this change and some documentation?

hi @sebasgarcep
I don't remember exactly what the problem was, but I think if we try to measure an empty view that collapse, it returns nothing. This is probably an edge case, not worth to be highlighted in main readme, but the user can find this info on this issue
      const [parentLayout, childLayout] = await Promise.all([
        this._measureElement(this._rnkasv_keyboardView),
        this._measureElement(element)
      ])

One idea would be to throw if childLayout is null/undefined here, with a link to this issue


Compile failed for the reason that the tag v1.12.8 has been deleted

Hi there,
The ojg is a JSON transform library that our product heavily relies on. We are extremely grateful for providing such a high-performance JSON parsing library. However, recently, many of our customers have encountered compilation issues when using an older version of our product. The problem stems from the deletion of the v1.12.8 tag.
Due to certain circumstances, our clients are unable to upgrade the version and tag of ojg. Could you please restore the original v1.12.8 tag? This is crucial for us. We understand that the situation is peculiar, as it involves an inability to upgrade to a newer version. However, the matter is urgent and unexpected. We greatly appreciate your help and support.
go: downloading [github.com/ohler55/ojg](http://github.com/ohler55/ojg) v1.12.8
src/server/transform/json/json.go:11:2: [github.com/ohler55/ojg@v1.12.8](http://github.com/ohler55/ojg@v1.12.8): reading<EMAIL_ADDRESS>404 Not Found
        server response: not found: [github.com/ohler55/ojg@v1.12.8](http://github.com/ohler55/ojg@v1.12.8): invalid version: unknown revision v1.12.8
src/server/transform/json/json.go:12:2: [github.com/ohler55/ojg@v1.12.8](http://github.com/ohler55/ojg@v1.12.8): reading<EMAIL_ADDRESS>404 Not Found
        server response: not found: [github.com/ohler55/ojg@v1.12.8](http://github.com/ohler55/ojg@v1.12.8): invalid version: unknown revision v1.12.8
make: *** [Makefile:36：mxgate] Error 1

many thanks.

Put the tag back. I highly encourage moving to a newer version though.

okay to close?


[Feature]: YML workflow file for auto labels

Is your feature request related to a problem? Please describe.
Instead of manually adding labels to issue for programs like gssoc, swoc etc. We need it automatic
Describe the solution you'd like
Make a new yml action file for this. Detect the word gssoc in description and add gssoc label. (Don't work on level based label)
Alternatives
No response
Additional Context
No response
Are you interested in contributing?

[ ] I'd like to work on this issue

Code of Conduct

[X] I have used the search function to check if an issue already exists
[X] I agree to follow this project's Code of Conduct

Hi @pranshugupta54 , I would like to work on this issue. Could you please assign the issue to me? Thanks.

@girish-07, assigned. Are you working under some program? (GSSoC, SWOC, Tublian etc?

Yes @pranshugupta54 , GSSOC


chore(ci): update workflow permissions to be least privilege

Was looking at the scorecard and saw simple enough fix.
Update token permissions to be least privilege.
Scorecard Details

{
  "name": "Token-Permissions",
  "score": 0,
  "reason": "detected GitHub workflow tokens with excessive permissions",
  "details": [
    "Warn: no topLevel permission defined: .github/workflows/add_issue_to_project.yaml:1",
    "Warn: no topLevel permission defined: .github/workflows/codeql-analysis.yml:1",
    "Warn: no topLevel permission defined: .github/workflows/go.yaml:1",
    "Warn: no topLevel permission defined: .github/workflows/release-drafter.yml:1",
    "Warn: topLevel 'packages' permission set to 'write': .github/workflows/release.yaml:9",
    "Warn: topLevel 'contents' permission set to 'write': .github/workflows/release.yaml:7",
    "Warn: no topLevel permission defined: .github/workflows/spelling.yml:1",
    "Info: jobLevel 'contents' permission set to 'read': .github/workflows/spelling.yml:77",
    "Info: jobLevel 'pull-requests' permission set to 'read': .github/workflows/spelling.yml:78",
    "Info: jobLevel 'actions' permission set to 'read': .github/workflows/spelling.yml:79",
    "Warn: jobLevel 'security-events' permission set to 'write': .github/workflows/spelling.yml:80",
    "Warn: no topLevel permission defined: .github/workflows/typos.yaml:1",
    "Warn: no topLevel permission defined: .github/workflows/updatecli.yaml:1"
  ],
  "documentation": {
    "short": "Determines if the project's workflows follow the principle of least privilege.",
    "url": "https://github.com/ossf/scorecard/blob/0276a7cd7284e077f22b50208345201485029fbd/docs/checks.md#token-permissions"
  }
}

I also let my IDE reformat yaml, let me know if you want the change without it.
Test
To test this pull request, you can run the following commands:
scorecard --local . --checks Token-Permissions

After

scorecard --local . --checks Token-Permissions
Starting [Token-Permissions]
Finished [Token-Permissions]

RESULTS
-------
Aggregate score: 10.0 / 10

Check scores:
|---------|-------------------|--------------------------------|------------------------------------------------------------------------------------------------------------------|
|  SCORE  |       NAME        |             REASON             |                                            DOCUMENTATION/REMEDIATION                                             |
|---------|-------------------|--------------------------------|------------------------------------------------------------------------------------------------------------------|
| 10 / 10 | Token-Permissions | GitHub workflow tokens follow  | https://github.com/ossf/scorecard/blob/49c0eed3a423f00c872b5c3c9f1bbca9e8aae799/docs/checks.md#token-permissions |
|         |                   | principle of least privilege   |                                                                                |
|---------|-------------------|--------------------------------|------------------------------------------------------------------------------------------------------------------|

Before

scorecard --local . --checks Token-Permissions
Starting [Token-Permissions]
Finished [Token-Permissions]

RESULTS
-------
Aggregate score: 0.0 / 10

Check scores:
|--------|-------------------|--------------------------------|------------------------------------------------------------------------------------------------------------------|
| SCORE  |       NAME        |             REASON             |                                            DOCUMENTATION/REMEDIATION                                             |
|--------|-------------------|--------------------------------|------------------------------------------------------------------------------------------------------------------|
| 0 / 10 | Token-Permissions | detected GitHub workflow       | https://github.com/ossf/scorecard/blob/49c0eed3a423f00c872b5c3c9f1bbca9e8aae799/docs/checks.md#token-permissions |
|        |                   | tokens with excessive          |                                                                                |
|        |                   | permissions                    |                                                                                |
|--------|-------------------|--------------------------------|------------------------------------------------------------------------------------------------------------------|

Additional Information
I think the only workflows that are not run in the context of PR are:

add_issue_to_project.yaml - Which uses a PAT
release-drafter.yml - Which uses permissions based on usage
release.yml - Which moved permissions from top level context to job level context.

Potential improvement
Might want to add some linting around it so it doesn't get reverted. There are linters like action-lint out there.

Might want to add some linting around it so it doesn't get reverted. There are linters like action-lint out there.

I love linters :D
Thank you very much for this fix.


Add licensing conditions for the OASIS 2.0 DITA schemas

There are two DITA OT plugins "org.oasis-open.dita.techcomm.v2_0" and "org.oasis-open.dita.v2_0". They come from the OASIS GitHub project which has other licensing terms.
Stefan our CEO looked over this aspect and would feel more comfortable with us distributing the DITA OT if you specified that those plugins have different licensing terms.
I quote from what he said:
In the NOTICES.txt maybe an entry can be added after line 41 which would mention the DITA 2.0 DTD, XSD and RNG schemas, mention their licensing terms, adding the licensing terms to a file named "oasis-dita20.txt" like they did for DITA 1.2 and 1.3. In that "oasis-dita20.txt" file maybe they can add the content of page 3 from this PDF: https://www.oasis-open.org/committees/download.php/65576/dita-2.0-specification.pdf

@robander does it make sense to add different licensing terms for the two plugins?

Here's what I did for the DITA OT bundled with Oxygen 23:
Added here:
plugins/org.oasis-open.dita.v2_0
this license file:
license.txt
and added information about DITA 2.0 schemas in the NOTICES.txt:
NOTICES.txt


Abnormal bandwidth usage

When I use the built-in remote desktop of Windows in Windows operating system, the bandwidth occupied is up to 10MB. When I use freerdp in Linux, the bandwidth occupied can reach 100M, and there is no remote desktop process built in Windows system. What should I configure?

well, best start with what you are doing ;)
your local setup, your command line, your remote os


Deprecate vfork

The compiler may generate incorrect code for vfork and setjmp because they are missing the #[returns_twice] attribute which is currently unstable (tracking issue). Since vfork is impossible to use safely, I propose deprecating it until #[returns_twice] is stable.

r? @gnzlbg
(rust_highfive has picked a reviewer for you, use r? to override)

Right now, because vfork is lacking the returns_twice attribute, the compiler might mis-compile user code in some cases. Do we have any reported mis-compilations (or MWEs) due to this that illustrate the issue?
Claiming that this feature is impossible to use correctly won't do much for users that are already using vfork, because if it works for them, then in their eyes, it can be used correctly, and also, there is no really an alternative available, so they will just turn the warning off.
So what do we want to achieve ?
I think the best we can do right now is document any known mis-compilations by filling an issue, and then use the deprecation warning to inform users that such known mis-compilations exist, and that there is a nightly feature that they can use to avoid them, but that they are kind of on their own for now.
Just telling them "don't use this" without telling them "what to use instead" produces a very frustrating user experience. If this is really always impossible to use correctly, we should at least point them to an issue that explains that in detail.

I haven't managed to create a miscompiling example, but essentially, if the compiler decides to perform tail call optimization in the vfork child, this will corrupt the entire stack frame for the parent as well.

Alternatively, the compiler could decide to share a single stack slot for 2 variables, one in the parent and one in the child since it "knows" that only one of the two branches is taken, not both.

I think maybe @comex had an actual mis compilation example for a lack of returns_twice attribute but I can find it. There is an issue open in the rfc repo for longjmp, maybe we could link there ? I'd prefer to avoid using the term "impossible", but I think the deprecation warning can be a bit longer and strongly suggest not to use this API and being clear that it is UB and that any new toolchain upgrade could break existing "working" code in subtle ways silently so using it is very dangerous.

I've managed to create a miscompilation example: https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=e192ab81e8d408fa9984f766e2356697
As you can see, variables modified in the vfork child cause unrelated variables in the parent to be modified.

However if you add #[ffi_returns_twice] then the issue no longer appears: https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=0de019e56b69aa048f3e0f5319cc187e

@gnzlbg ping?

This looks good to me @Amanieu. CI is red due to unrelated issues, but I'll merge it ASAP.
In the mean time, could you create an issue in this repo explaining the problem with those links, and add a link to it to the deprecation message ?

Added a reference to #1596 in the deprecation message.

Thanks, as soon as we get up bors working again for the repo I'll start merging PRs.

@bors: r+

:pushpin: Commit 1f7352ca3dc3dd4717d24267ec56cca85def0a40 has been approved by gnzlbg

:hourglass: Testing commit 1f7352ca3dc3dd4717d24267ec56cca85def0a40 with merge a13ad69c4277a67bb1b1f89d6eeb0bc562d5a590...

:sunny: Test successful - checks-cirrus-freebsd-10, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, status-azure
Approved by: gnzlbg
Pushing a13ad69c4277a67bb1b1f89d6eeb0bc562d5a590 to master...


Fix signing of whole transaction, prevent repeated passphrase prompts

See commit it release branch to see how to fix this properly :)


Hosting simple website

Missing labels.

Codecov Report

Merging #299 into master will increase coverage by 1.14%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #299      +/-   ##
==========================================
+ Coverage   38.47%   39.62%   +1.14%
==========================================
  Files          88       99      +11
  Lines        2773     4747    +1974
  Branches        6        6
==========================================
+ Hits         1067     1881     +814
- Misses       1706     2866    +1160

@jakubkornafel you forgot to delete a branch


[Experiment] Reformat issue template.

Nagging Assignee @yifeif: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.


6.2 content additions planning

With more of us contributing here, it Seems To Me to make sense to do a bit of a roll call for those of us who do regular raidboss/oopsy work so we don't step over each other. I don't really care which piece of content I do aside from "I'd prefer not to do the trial".

MSQ Dungeon
MSQ Trial
EX Trial -- Legends0
Pandaemonium 5
Pandaemonium 6
Pandaemonium 7
Pandaemonium 8
Sephirot Unreal -- JLGarber

I'm envisioning each of us noting what we're going to focus on, possibly sharing logs to whoever has a piece of content that we're running. I'll start by claiming the Unreal, since I already did the EX version. I'll also pick some other stuff up in a day or so if not everything is in progress, since the Unreal should be a simple find/replace for the most part.
I know of at least @valarnin @Legends0 as well as @quisquous who regularly do Raidboss stuff. Maybe @panicstevenson as well. Others please also add in your preference on content once we see stuff released!

Thoughts:

Chime in here when we start working on something. Follow up as needed

Kind of clutters up a single issue
Makes it hard to track comments by content

Use this issue as a reference, open a separate issue mentioning this issue when we start working on the content

Lets this issue serve as a common tracking point for the patch
Can reference the separate issue in the PR for auto-closing
Problems/thoughts/brainstorming tracked per content, easier to follow and chime in if someone has feedback/help

Additionally, I think a super early PR that adds skeleton/placeholder files and folders for timeline and trigger files would be a good idea to avoid conflicts in manifest?

I agree with having skeleton files in place. Makes it easier to setup and start testing since I often don't have the zone ids.
I likely won't have anything for the first week here and likely not have anything for the first day as well. I tend to focus in on the harder content first, which can be relative. No plans to do the dungeons or unreal content. If I did anything for the first day here it may just be ex and might just be a timeline.

Zone ids are updated, and skeleton files should all be added now.

PR up for EX trial #4689. Feel free to make edits to it as I will be away.

Not sure if it should be a separate issue or not, but the second "knockback" timeline trigger callout seemed to be late for me (it was right when the knockback went off).

Not sure if it should be a separate issue or not, but the second "knockback" timeline trigger callout seemed to be late for me (it was right when the knockback went off).
(Come to think of it, it has a cast, so doesn't need to be a timeline trigger at all?)
There are three spells that go off, the timeline syncs to the insta cast one as it was consistent. It's possible to switch to the casted ones, but would need all their Spell ids and a supressSeconds. At the time I just went with the insta cast which needs timeiline trigger. It's interesting that it worked for the first but was late for the second... might be a jump in the timeline occurring that could cause this?

75A0 and 75A1 seem to start casting at the same time and have the same duration, so either one should work. 75A1 is the one that actually hits people while 75A0 hits nothing, but it shouldn't matter for the purposes of the trigger.

I will take a look at P5N

My group is running the Extreme tonight and guaranteed someone won't have the normal mode done so I'll get that done sometime tonight.

Closing since we have done basically all the content that might cause duplication of work.


Mark ol.Observable.prototype.unByKey deprecated.

The ol.Observable.prototype.unByKey was supposed to handle removing of
listeners registered using ol.Observable.prototype.on and
ol.Observable.prototype.once.
However, the underlying google library code called by unByKey only requires
the key, not the ol.Observable object on which the registration was
triggered. Moreover, this method was unconvenient and in practice misused.
Example or real world code:
var map = new ol.Map();
var key = map.on(listener);
view.unByKey(key);

A function was added in #2794, in particular to fix this kind of code.
This PR pushes forward the removal process of the old method in favor of the
unByKey function by marking it @deprecated and pointing to the right
usage.
Please merge when deemed appropriate.

I don't think we should mark this as deprecated, much less say we are going to remove it (and that wouldn't happen anyway before 4.0).
We're facing a huge challenge in making it easy for people to discover what's in the library.  The ol.Observable.unByKey function is arguably harder to find than the unByKey method, and I don't think there is a good argument for removing, deprecating, or otherwise making the unByKey method even harder to discover.  In addition, when generating a custom build, I imagine people will make use of wildcard exports until we have an extremely user-friendly and flexible hosted build tool.  Exporting ol.Map#* gets you everything you need to work with a map.  Having to discover ol.Observable.unByKey and export that as well is an additional (unnecessary challenge).
So, since we've already got the unByKey method and we're not going to break the stable API, I'm opposed to making it any less convenient to use.

The ol.Observable.unByKey function is mentioned in the ol.Observable.prototype.on method  and similarly easy to discover as the prototype version and it prevents misleading uses as explained in my previous comments.
I am confident people able to generate a custom build are able to export the unByKey function.
That being said, since there is no removal policy before 4.0, I am OK seeing my PR rejected.


[HOLD for payment 2022-02-16] Group creation - Search box is not cleared after selecting a user

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:

Shift+cmd+k or :green-plus-button: to start new group chat
Start typing name of user
Press enter to select user or click on user's name with cursor
Observe text from search for user is still in search box.

Expected Result:
Search box should clear after selecting a user.
Actual Result:
Search box is not cleared after selecting a user
Workaround:
User can delete the content in the search box.
Platform:

Where is this issue occurring?

Web
iOS
Android
Desktop App
Mobile Web

Version Number: 1.1.23-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
https://user-images.githubusercontent.com/44479856/147013069-34733ba5-05c7-4734-9912-1955c15b2290.mp4
Expensify/Expensify Issue URL:
Issue reported by: @mallenexpensify
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1640130861454000
View all open jobs on GitHub

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

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

Proposal
Here, on selecting a row the searchValue is changed(change of state) to ' ' and on deselecting a row(i.e, removing selected member) searchValue remains as it is, i.e,
searchValue: isOptionInList ? prevState.searchValue : ' '
But, the changed searcheValue is not reflected from BaseExpensiTextInput as instead of using this.props.value while calling TextInputWithName Component,  this.value is being used.
So, changing
 <TextInputWithName          ----          value={this.value}          ---           />
to
 <TextInputWithName          ----          value={this.props.value}          ---           />
in BaseExpensiTextInput will solve the issue.

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

Reassigning external as I'll be ooo for the next 2 weeks, thanks @jliexpensify!!

Posted!
Internal - https://www.upwork.com/ab/applicants/1474202959761481728/job-details
External - https://www.upwork.com/jobs/~01eb6b3ea2ed5a0a26

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

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

@jliexpensify
Here is my proposal for this issue
Proposal
I have added this new else if block
else if (!this.props.value) {       this.input.focus();       this.setState({isFocused: true});     }
in componentDidUpdate method.
Updated componentDidUpdate  code

Videos
Android
https://user-images.githubusercontent.com/5493716/147464959-9199e507-b07c-44c6-965f-5eedc73130e1.mp4
Web
https://user-images.githubusercontent.com/5493716/147465385-aef1593b-ea69-473f-9b58-39ed7d4dd57a.mov
Does this make sense?

@sobitneupane Are you suggesting to make the ExpensiTextInput as controlled?

@parasharrajat Now, BaseExpensiTextInput is structured as a controlled component but update of on change text in being maintained in class variable. So, though the state of textinput was updated from NewChatPage.js considering BaseExpensiTextInput to be a controlled component, the effect is not reflected. So by using state instead of class variable, we can make BaseExpensiTextInput controlled.

Thanks, @sobitneupane for clarification. I think It is fine to do. But I would like to discuss this in slack as this would affect all inputs in the app.
If we do this, there are a few other places where we have to do a small clean-up. I will point out those once we moved with your solution.

Okay @parasharrajat . Should I message you in slack?

Sorry, I meant I will discuss this on the public channel.

https://expensify.slack.com/archives/C01GTK53T8Q/p1640793789214900

Thanks for waiting on this. Unfortunately, keeping in mind that controlled input has performance issues, I can't accept the proposal that makes it controlled.
@sobitneupane Do you have any other solution to fix it by making it controlled? It is fine even if it involves refactoring the BaseTextInput.

To clarify further, as we are storing the value in class variable instead of a state, value of input is being changed or controlled but not updated in the react component. The changed value is passed from OptionsSelector to BaseExpensiTextInput. So, If we select two options for the same search value, for the second selection, the input value is changed to ' ' as the input value was updated to ' ' in previous step.
The other way around is to use this.props.value in place of this.value in BaseExpensiTextInput.
<TextInputWithName
         ----
         value={this.props.value}
         ---
/>

Well, I understood your proposal. I will get back to you.

Ok. Sorry for the delay. We are not ready to make BaseExpensiInput Controlled. @sobitneupane I understood your proposal but we can't do that do you have any other approach to solve this issue.
Can we try with ref.clear() as the last resort or trigger an update on the input so that it clears the input automatically?
Or Do you think of a way so that BaseExpensiInput can be improved by making it controlled?
I would prefer Do you think of a way so that BaseExpensiInputcan be improved by making it controlled?

We can clear the input when the row is selected by using ref.clear() in OptionsSelector.js.
selectRow(option) {
        if (this.props.shouldFocusOnSelectRow) {
            this.textInput.focus();
        }
+        this.textInput.clear();
        this.props.onSelectRow(option);
    }

But for this to work we should also remove value={this.value} from BaseExpensiTextInput.js. And I don't think it will have major impact on any component using ExpensiTextInput as it is expected to be uncontrolled by other components as well.
<TextInputWithName
          ref={(ref) => {
              if (typeof this.props.innerRef === 'function') { this.props.innerRef(ref); }
              this.input = ref;
          }}
          // eslint-disable-next-line
          {...inputProps}
-         value={this.value}
          placeholder={(this.state.isFocused || !this.props.label) ? this.props.placeholder : null}
          .
          .
          .
 />

But for this to work we should also remove value={this.value} from BaseExpensiTextInput.js. And I don't think it will have major impact on any component using ExpensiTextInput as it is expected to be uncontrolled by other components as well.

Could you please explain it to me Why?

But for this to work we should also remove value={this.value} from BaseExpensiTextInput.js.

It should be removed because value = {this.value} will reset the value if not removed.

And I don't think it will have major impact on any component using ExpensiTextInput as it is expected to be uncontrolled by other components as well.

And It should not impact other components using it because this value is being used to take input from the parent component but as we are using class variable instead of state variable, value is not being updated.

So if we remove this how are we supposed to pass the value to ExpensiTextInput. just trying to understand your idea.

No, we won't be able to pass value from parent component to ExpensiTextInput (as we are expecting it to be uncontrolled).

Sorry for the delay, I will share this internally to get more eyes.

proposal
The problem is in this line
https://github.com/Expensify/App/blob/13b7b0b1945989379ab63f098ee2f480e1b1ba4b/src/components/TextInput/BaseTextInput.js#L183
value should be this.props.value and not this.value. Most recent value for textinput is in this.props.value and not in this.value. There is additional steps to set this.value.

proposal
The problem is in this line
https://github.com/Expensify/App/blob/13b7b0b1945989379ab63f098ee2f480e1b1ba4b/src/components/TextInput/BaseTextInput.js#L183
value should be this.props.value and not this.value. Most recent value for textinput is in this.props.value and not in this.value. There is additional step to set this.value.
set value for this.value in compponentDidUpdate:
https://github.com/Expensify/App/blob/13b7b0b1945989379ab63f098ee2f480e1b1ba4b/src/components/TextInput/BaseTextInput.js#L55
This will not trigger render to render the text input;
Or there is other method: by calling this.setValue inside componentDidUpdate. But  something weird happen in the group OptionSelector

How about changing below line with this.setValue(this.props.value) in:
https://github.com/Expensify/App/blob/991504fd8aefe8674f045bca11ae3c31f87337b2/src/components/TextInput/BaseTextInput.js#L55
And all this.value.length check condition must be coupled with this.value. So it will be this.value && this.value.length.
In this:
https://github.com/Expensify/App/blob/991504fd8aefe8674f045bca11ae3c31f87337b2/src/components/TextInput/BaseTextInput.js#L103
and this:
https://github.com/Expensify/App/blob/991504fd8aefe8674f045bca11ae3c31f87337b2/src/components/TextInput/BaseTextInput.js#L115

Also, doubled row option that you see after selecting a user (after this change) might be related to other bug about doubled option in group search.
The bug appear if you enter a text to search and then delete character with backspace. Happen in production.

Proposal
I have noticed that the issue is caused when the value is changed programmatically from outside. The input field needs to be focused for the update to take effect. Notice from the video that when the  searchValue is changed
https://github.com/Expensify/App/blob/0bce09177d2aed8c5e377bc758f5241dd22886eb/src/pages/NewChatPage.js#L188
The input field does not get updated. Yet when you navigate   away from the app  and back the input field automatically gets updated with previous changes.  So placing
        this.input.focus();

below
https://github.com/Expensify/App/blob/0bce09177d2aed8c5e377bc758f5241dd22886eb/src/components/TextInput/BaseTextInput.js#L55
does the trick .
Video
https://www.awesomescreenshot.com/video/6987681?key=c7d082e190c84cc093e339551e808522

@brianmuks That's interesting!
@parasharrajat Was there a conclusion in the discussion?

@pecanoro No. Nobody seems to be interested 😅. Could you please reshare the thread in the Channel?

Okay. There is no update on slack after pushing the discussion for several days. It seems to me that it is time to take the matter in my :open_hands: .
@K4tsuki you have an interesting solution https://github.com/Expensify/App/issues/6872#issuecomment-1013060140. But let's demisify this.

we send a value via props.
value is received as ComponentDidUpdate.
SetValue change this.value and you again push an update to the parent component via OnChange event.
It means we receive a value from the parent and then you again send an update to the parent.
Let's say a parent is passing the value from state and when the user types, it would update its state. Right? So onChange it updates the state. Now, it will send the value to TextInput via value prop and then the parent will do point 3 and loop continues.

Am I missing something?

At this stage, I would prefer to set the value to this.props.value and clean up the component. Thus I like @sobitneupane 's proposal.
@pecanoro What do you think?

Please wait...

Let's take a look at real usage here:
https://github.com/Expensify/App/blob/07582967349266b430103b11a94839bf40d8bccb/src/pages/settings/Profile/ProfilePage.js#L217-L224
Component FullNameInputRow inside ProfilePage use this 'TextInput' component.
There is FirstName text input value set by this.state.firstName.
Also there is onChangeFirstName that listen to onChangeText on first name that will setState of firstName each time the onChangeText being called.
This is just like what you described.

The data flow when user type a character inside firstName textInput is:

onChangeFirstname being called
Profile page getting rendered, because state of firstName changed inside this function
BaseTextInput rendered/updated
on componentDidUpdate of BaseTextInput passing check on early return and then setValue being called => onChangeFirstname called again, back to step 1 until 3 and then early return happen in:
https://github.com/Expensify/App/blob/07582967349266b430103b11a94839bf40d8bccb/src/components/TextInput/BaseTextInput.js#L51-L53

yeah, I got your point but I leaning more toward I would prefer to set the value to this.props.value.

It will become controlled.
For controlled TextInput you can use import {TextInput} from 'react-native'; Not this TextInput inside element.
Please respect decision from @marcaaron and other in slack.

There is no conclusion on slack yet.
AFAIK, our TextInput is already controlled. Using value prop makes TextInput controlled. It does not matter if it is an instance or state property.  https://expensify.slack.com/archives/C01GTK53T8Q/p1642096772440500?thread_ts=1640793789.214900&cid=C01GTK53T8Q

Please respect decision from marcaaron and others in slack.

It is my opinion and I have asked the concerned person. I would be thankful if you be patient.

I believe there are performance issues related to having inputs be controlled and that’s why we are planning to standardize on uncontrolled.

This is what @marcaaron said in slack.

Ok, let me revive the conversation in Slack since I need the opinion of the most knowledgeable React Native Expensify engineers, I am stronger on the back-end side so I can't see all the ramifications of making the component controlled.

Or we can explicitly clear the input

That is what Marcaaron emphasized in slack.
So we need a reference to the TextInput call clear() method. Or using setNativeProps ({text: ''})

Yeah, that can be an option. Maybe we can remove the value prop and use defaultValue instead of it with this if that works.

Ok, seems like this needs a push so I took a deeper look.
Yes, the input is controlled (sort of..) we are doing a lot of weird stuff and the custom TextInput component is pretty complex so I think we're both kind of right, sorry for the confusion @parasharrajat.
Given where we are at, I think it is fine for the TextInput to work in a controlled way wherever it is used. But it's not working correctly. We are missing a step which is that if props.value === '' then we must programmatically clear the input.
I'm not sure if input focusing is on the right. This issue is only mentioning the input clearing not suggesting we bring the focus back to the input. That seems like a separate issue maybe or working because of a side-effect.
So, the issue is that the TextInput does not clear if the value changes and we have passed an empty string for the value.
Why does it happen? The answer is...
We only handle the change in value when the component updates. Which happens after the render. So it is a controlled component, but not really as we might expect as the value is only passed after the component renders. Here's the typical case of adding some text to the input...

User changes text ->
TextInput value changes ->
change handler calls this.props.onChange ->
parent passes a new props.value ->
that value DOES NOT get passed to the component in the render() but the value has changed on screen ->
componentDidUpdate() handles the new value and sets this.value ->
that does not trigger the component to re-render (but again, the value has changed on screen so all is fine and the value we see is synced with the value in this.value)

Let's go through the same flow, but test selecting a user

User adds a participant ->
props.value == '' is passed to TextInput ->
TextInput render()
TextInput sees that the value changed and update this.value to an empty string ->
But the value on-screen does not change... 💥

It doesn't change because the component is not re-rendering (required to pass this.value to the underlying TextInput). To make this work correctly I think we can just do something like
diff --git a/src/components/TextInput/BaseTextInput.js b/src/components/TextInput/BaseTextInput.js
index c95856b0b..239766c5f 100644
--- a/src/components/TextInput/BaseTextInput.js
+++ b/src/components/TextInput/BaseTextInput.js
@@ -53,6 +53,9 @@ class BaseTextInput extends Component {
         }

         this.value = this.props.value;
+        if (this.props.value === '') {
+            this.input.clear();
+        }

         if (this.props.value) {
             this.activateLabel();

If that works OK, can we do it and follow up to improve this component later?

@K4tsuki seems like you were the closest to this solution. Everyone else agree with that and are we good to assign them?

Thanks for investigating @marcaaron. I am glad that we have a mutual agreement on what should be done.

follow up to improve this component later?

I am working on cleaning up our TextInputs https://github.com/Expensify/App/issues/6584. I can add another TODO there.
yeah, @K4tsuki was closet. He suggested using So we need a reference to the TextInput call clear() method. Or using setNativeProps ({text: ''}) .
I like the latest suggestion from Marc.
cc: @pecanoro
:ribbon: :eyes: :ribbon:  C+ reviewed

Thank you for the great explanation @marcaaron.
@marcaaron this is works for this.props.value == '', but for value other than = ''. It still doesn't work.
I have tested in NewChatPage constructor, windows setTiimeout to change searchValue state and it is not changing.
In componentDidMount of BaseTextInput filter check should be:
if (this.value === this.props.value) in:
https://github.com/Expensify/App/blob/991504fd8aefe8674f045bca11ae3c31f87337b2/src/components/TextInput/BaseTextInput.js#L51
This will filter out changeText by user typing, because onChangeText of BaseTextInput is setting this.value and calling consumer onChangeText. So on componentDidUpdate of BaseTextInput, this.value is same as this.props.value so we must filter it.
After  this.value == this.props.value; we could call  this.input.setNativeProps({ text: this.props.value});

If I am really assigned to this, I want to divide the reward with Brianmuks and Sobitneupane . I think proposals are close and we have been working hard for this (If I really assigned to this).

@parasharrajat I am sorry I thought you are jumping to the conclusion to change value={this.value} into value={this.props.value}

@marcaaron this is works for this.props.value == ''

Yes, when selecting a user the props.value is set to an empty string and we need the TextInput to clear

but for value other than = ''. It still doesn't work.

What doesn't work exactly?

I want to divide the reward with Brianmuks and Sobitneupane

That's very kind. Not sure anyone has asked this before, but can relay this feedback to the team.

What doesn't work exactly?

Other than empty string.

And shouldn't this check:
https://github.com/Expensify/App/blob/991504fd8aefe8674f045bca11ae3c31f87337b2/src/components/TextInput/BaseTextInput.js#L51
be if (this.value === this.props.value)
because this.value changes when user is typing but doesn't change when consumer of BaseTextInput set the props.value.
and then we can call this.input.setNativeProps({ text: this.props.value});?

Ah, I see your point now @K4tsuki. To clarify

When the user typed acids.
On the Consumer(parent component) we blocked the update of value as we are only accepting numbers.  So value will be ''.
Now on the TextInput the this.value === acids but this.props.value === '' and also the prevProps.value would be '' thus componentDidupdate will early return.

but the textinput will keep acids as this.value is till acids.
So you are suggesting we use this.input.setNativeProps({ text: this.props.value}); to update this. I agree this should be used to update uncontrolled.
But I tried and change is not spontaneous. textInput only update when we blur the focus from it.

So we will have to force update. Do you see any other solution that problem?
--- a/src/components/TextInput/BaseTextInput.js
+++ b/src/components/TextInput/BaseTextInput.js
@@ -46,13 +46,15 @@ class BaseTextInput extends Component {
         this.input.focus();
     }

-    componentDidUpdate(prevProps) {
+    componentDidUpdate() {
         // activate or deactivate the label when value is changed programmatically from outside
-        if (prevProps.value === this.props.value) {
+        if (this.value === this.props.value) {
             return;
         }

         this.value = this.props.value;
+        this.input.setNativeProps({text: this.value});
+        this.forceUpdate();

         if (this.props.value) {
             this.activateLabel();

I test using
https://github.com/Expensify/App/blob/2cdb7c30be70766b3c006a67153665d895ba83ac/src/pages/NewChatPage.js#L188
Instead of empty string try to change in to lorem ipsum to be : searchValue: isOptionInList ? prevState.searchValue : 'lorem ipsum',
And it works without forceUpdate()
Where is it doesn't work?
In the same new group chat page?

Ok, then it's fine. we can leave the forceUpdate().

Similar to what you write above, but without foceUpdate
--- a/src/components/TextInput/BaseTextInput.js
+++ b/src/components/TextInput/BaseTextInput.js
@@ -46,13 +46,15 @@
    class BaseTextInput extends Component {
         this.input.focus();
     }

-    componentDidUpdate(prevProps) {
+    componentDidUpdate() {
         // activate or deactivate the label when value is changed programmatically from outside
-        if (prevProps.value === this.props.value) {
+        if (this.value === this.props.value) {
             return;
         }

         this.value = this.props.value;
+        this.input.setNativeProps({text: this.value});

         if (this.props.value) {
             this.activateLabel();

(this.value === this.props.value) this check is filters user typing in text input, so  we don't call setNativeProps needlessly,

because this.value changes when user is typing but doesn't change when consumer of BaseTextInput set the props.value.

Hi, can we wrap this up, please? I tested the solution I shared and it seemed to work. Is there some problem with it and does it not fix the issue? Remember, we are trying to fix this issue not some other issue or problem we may have identified.
What is the point of using setNativeProps()?

@marcaaron If we solve this current issue then using your suggestion is best.
It is used for other problem. I have tested it using lorem ipsum above, then the search box doesn't display lorem ipsum after select a user.
It means if props.value is supplied to BaseTextInput there is still a problem of not being displayed.

Sounds good! I'm not sure what other problem we are trying to solve, but if there is one feel free to report it.

All right let's wrap this up and using @marcaaron suggestion above.
Thank you @marcaaron

I'm assuming we're hiring @K4tsuki ?

Yeah, I agree. I faced the problem that @K4tsuki mentioned in my PR  and the these  reproduction steps will match with the issue that I faced on that PR. Thus I pointed out here as @K4tsuki found the cause of the issue. This solution fixes the issue on that PR and requires force update as well.
As Marc said, we can just focus on fixing this GH issue and I will fix the other that I faced on m PR which will require similar solution  to https://github.com/Expensify/App/issues/6872#issuecomment-1024812272

Thank you for the discussions, everyone, it took me a bit to catch up but I got to learn a lot about this flow. Assigning @K4tsuki since we all agreed it seems to be the best approach and it seems like a good solution to the best of my knowledge.

📣 @K4tsuki You have been assigned to this job by @pecanoro!
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 📖

@K4tsuki could you link me your Upworks profile please? Thanks!

@jliexpensify I want  to divide the reward for the three of us: me, brianmuks and sobitneupane as I state above. So the reward  is divided into three.
Here is my upwork profile:
https://www.upwork.com/freelancers/~01bdd9e3a6f15042bd
Thank you.

@K4tsuki Unfortunately, we don't really have a system to facilitate this. If you want to share the fee then that is up to you, but we can only hire one person to complete the job.

@marcaaron all right, I will follow the system.
And will contact sobitneupane and brianmuks later.
Thank you.

@parasharrajat @pecanoro - just wondering: am I good to pay this one? It looks like there was a failure on the iOS end

Yeah, @jliexpensify It is deployed. That was some other glitch.

Cheers, paying now:

@K4tsuki $250
@parasharrajat $250

Will close once done


Fix selection visibility

This one fixes https://github.com/cannikin/twilight-syntax/issues/8 issue. Which wasn't fixed by previous revert.

Published v0.7.0


UN-145: Implement front end design updates to individual Insight page

Hi @ababic,
Here's the new PR for the updates to the Insights FE. Just give me a shout if you need anything 👍

Hi @ababic,
Apologies for the delay, I was sidetracked by a few things that took longer than expected. Thanks for looking at this and your comments, I've made those changes now :)


Add a placeholder for format template for all contributors

Background
Possibility to format the Releae note output format with placeholders is great.
Feature
Add a placeholder into format template that represents all contributors. A combination of developed-by and co-authored-by

@benedeki What leading string would you expect here?


COCO-Stuff Evaluation

Hello, I wanted to understand the evaluation protocol you are using for COCO-stuff
It seems in https://github.com/sinahmr/NACLIP/blob/0cac3a651f753b11315ea799cfbfabf79da1bd76/datasets/cvt_coco_object.py#L192-L198
You are setting all stuff categories to background. In that case how is COCO-Stuff different from COCO-obj ?

Hi,
Yes, you are right.

I see, so the results reported in table 1 in the paper under C-Stf only include "thing" categories ?

C-Sft evaluates on all thing and stuff classes, while in C-Obj, we only have thing categories and a background class.


Extract new "Close issue" dropdown buttons

Description
Like we do in one-click-pr-or-gist

(not sure why this screenshot looks so bad, it should be updated)
Screenshot

Example URL
Any open issue you can close, like this one

Seems like nobody noticed that the following issue was a duplicate:

#6883

Anyway, it's fixed in https://github.com/refined-github/refined-github/releases/tag/23.11.1:

#6973


[mesheryctl] Shorten the User ID in app list command output

See mesheryctl Command Tracker for current status of commands.
Current Behavior

in mesheryctl app list command user full UserID is shown.
APP ID          USER ID                                 NAME                                    CREATED         UPDATED
1503c253        a550f4d6-d3bd-44f1-9b4c-167114452062    prometheus-elasticsearch-exporter-5.1.1 4-14-2023       8-4-2023
e5965720        6a6ccef1-39d0-4ac4-8ae1-dc47d455d7ff    redis-deploy.yaml                       8-6-2023        8-6-2023
b7d3e14b        6a6ccef1-39d0-4ac4-8ae1-dc47d455d7ff    currency-deploy.yaml                    7-8-2023        7-20-2023

   TOTAL                     3

Desired Behavior

Show only first segment of User ID in output when --verbose flag isn't passed.
APP ID        USER ID       NAME                                            CREATED         UPDATED
1503c253      a550f4d6      prometheus-elasticsearch-exporter-5.1.1         4-14-2023       8-4-2023
e5965720      6a6ccef1      redis-deploy.yaml                               8-6-2023        8-6-2023
b7d3e14b      6a6ccef1      currency-deploy.yaml                            7-8-2023        7-20-2023

   TOTAL                     3

Implementation

this issue is same as #8250
Acceptance Tests

Mockups

Contributor Resources

⌨ mesheryctl Contributing Guide
Beginner's guide to contributing to Meshery and mesheryctl
mesheryctl Command Tracker
Meshery CLI Commands and Documentation
🙋🏾🙋🏼 Questions: Layer5 Discussion Forum and Layer5 Community Slack

Can I work on this please?

@gopivaibhav go ahead!

@gopivaibhav go ahead!

Thank you @zakisk , I have made the PR :))


JS has only made partial progress to catch up to UIKit

This happens on navigation if using the NavigationBar back button semi-quickly:

Dup of #731 :smile:


Treat literal '.' literally (fixes #627)

Fixes #627: Bug: Literal pattern '.' is not treated literally

Nice find. Thanks for fixing it.


Build & Test With Jest on Github Actions Every Branch

This establishes a process for automatic building and testing on every commit. That way everyone involved can know that high code quality is always maintained.
Next steps can include:

automatically run next lint to ensure code quality
add more tests for hooks, utilities
add live end to end testing of page features
build and zip archive of nextjs app to releases
build and push docker containers
different build settings for different branches
enforce github actions succeed to merge pull requests

@rxxndy what do you think of this approach?

@owenkellogg I like the idea of this. Give me some more time to read through this first

@owenkellogg do you know if there is a way to ensure prettier is run on every commit? Above test, this is always something that irritates me if i open up a file and its not formatted

@owenkellogg do you know if there is a way to ensure prettier is run on every commit? Above tests, this is always something that irritates me if i open up a file and its not formatted

Yes, you can ensure that prettier (or any other code formatting tool) is run on every commit by using a combination of husky and lint-staged. These tools allow you to automatically run scripts at different stages of your git workflow, such as before committing. This setup can help enforce a consistent coding style by automatically formatting staged files according to your prettier configuration.
Here's a step-by-step guide to setting this up in your project:
Step 1: Install the Necessary Packages
First, you need to install husky, lint-staged, and prettier as development dependencies in your project. If you haven't installed prettier yet, you'll need to add it too. Run the following command in your project root:
npm install --save-dev husky lint-staged prettier

Or if you're using Yarn:
yarn add --dev husky lint-staged prettier

Step 2: Set Up husky and lint-staged
After installing the packages, you need to configure them. This usually involves editing your package.json file to include configurations for husky and lint-staged.
Configure lint-staged
Add a lint-staged configuration in your package.json that specifies running prettier on all staged files. You can also add other commands to lint your code in the same step if you wish.
"lint-staged": {
  "*.{js,jsx,ts,tsx,json,css,md}": [
    "prettier --write",
    "git add"
  ]
}

This configuration will run prettier --write on all staged files matching the specified patterns and then add the changes to the commit.
Configure husky
Set up a husky hook to run lint-staged before each commit. You can do this by adding a husky configuration in your package.json:
"husky": {
  "hooks": {
    "pre-commit": "lint-staged"
  }
}

This configuration tells husky to run the lint-staged command before every commit, which in turn runs prettier on your staged files.

Note

The "git add" command in the lint-staged configuration was necessary in older setups to re-add the files after prettier formatted them. However, newer versions of lint-staged automatically add modified files back to the git staging area, so you might not need to explicitly include "git add" depending on your lint-staged version.
Ensure you have a .prettierrc file or equivalent configuration for prettier in your project so that prettier knows how to format your files.
If you're setting this up in an existing project with multiple contributors, it's a good idea to inform them about the new pre-commit hook. They'll need to run npm install or yarn install to install the new development dependencies and set up the hooks on their local machine.

https://typicode.github.io/husky/how-to.html


Folders not ignored when added to .watchmanconfig in source folder

I'm developing a node module and want to exclude a examples/ directory in the module folder.

I added "examples" to the "ignore_dirs" list in the module/.watchmanconfig file.
I added the wml task, with wml add source/path/to/module /target/path/to/module
Did wml start

And still, it copies over examples/. Which is a big folder.
After 2., wml add still asked me if i wanted to ignore .git and node_modules. But these were already in .watchmanconfig.
Am I doing something wrong? Should I not edit .watchmanconfig?
Thanks for any help!

same problem here.
looks like watchman doesn't update your watch list after edit .watchmanconfig.
This steps work fine for me:

edit .watchmanconfig and say what directories you want to ignore
clean watchman list running command watchman  watch-del-all, you can see the full list running watchman watch-list
and finally wml add and now you directories will be ignored

@grisaitis this package is no longer maintain and has a lot of bugs.
you can use this package
https://www.npmjs.com/package/mtsl
mtsl add -s ~/source-dir -d ~/User/destination-dir
mtsl start 0

this package is using chokidar instead of watchman

Thanks @nomi9995
Closing out


hotkeys: Add x for composing to PM.

It should do the same thing that C currently does. Remember to also update the on hover instructions, and the keyboard shortcuts help.
All traces of C should be removed from the documentation/help. Check or ask on https://chat.zulip.org/#narrow/stream/design/topic/pm.20hotkey to see what the consensus ends up being on whether to allow C to continue working in the code.

Sounds good! And yeah, a modal popup or something similar to our "you've just muted a topic" popup would be pretty reasonable if we do decide to get rid of C.

@zulipbot claim

@zulipbot claim

It appears that #6592 replaces this hotkey, without any legacy support for the old key (or warnings).  Which I personally think is fine.

I think we should either include legacy support for the old key or do a warning.  This is one of the most-used hotkeys, and I think many of our major users will think it is a bug if it just stops working.

@rishig  @timabbott @showell  Shall I work on this issue?

@zulipbot  claim

@zulipbot  abandon

@zulipbot claim

@zulipbot abandon

@verma-varsha are you still working on the issue?

The modal only shows if you accidentally type the deprecated key.
I guess that doesn't work though for hotkeys that change, rather than being deprecated. We can worry about that later though, I think.

@zulipbot claim


Use a common entry.cpp with tag dispatch to replace per-layer entry.cpp

The current code uses C++ template tag dispatch for the main layer intercept/driver dispatch function pointer tables, but not for the implementations backing the exported entrypoints. We currently emit per-layer code that is hard-coded to call "_default()" implementations. This is functional and allows replacement in the layer, but results in more code to maintain over the layers.  We should just use the template tag dispatch mechanism everywhere.

Also, now that we have tag dispatch we should move the remaining default implementations that are built by the generator out of the text stubs that are injected into instance_functions.cpp and put them into normal
C++ code that is maintained in instance_functions_manual.cpp.


Add a secondary dns forwarder IP

The second commit is the interesting one. Its commit log:
In #38 I attempted to address the problem of flakey upstream DNS by
round-robinning DNS retries (detected via the DNS ID field), replicating the
fallback behaviour a client which has been given multiple entries in
resolv.conf despite only providing then with a single entry.

However this turns out to have been insufficient because not all client side
resolvers reuse the DNS ID on retry, so we do not see those retries as such and
send them to the primary upstream DNS a second time.

To address this create a second IP end point (on the same underlying device, so
essentially an IP alias)  and expose it to the client (via DHCP) as a second
DNS server which forwards to the secondary upstream DNS server and revert most
of #38 such that for the existing local IP we always forward to the primary
upstream DNS server (if the host only has a single DNS server configured then
both end points will go to the that one).

Third and subsequent host DNS servers are ignored.

This has only been tested on Mac, but given an implementation of Resolv_conf
which returns 2 (or more) DNS end points this should work for Windows too.

This second IP address is bolted onto the side in what appears to be a slightly
odd way. However I think it is consistent with the direction (AIUI) we intend
to be moving which is to remove the use of `Tcpip_stack_direct` (since we
aren't really a TCP/IP "endpoint") in favour of a lower level approach (based
on the newer Marshal/Unmarshal interfaces in mirage-tcpip rather than even more
open coding). Currently mirage-tcpip does not support aliases (second IPs) on a
device, so going the route of creating a second full stack would have required
an entire extra network device be exposed to the client.

LGTM

Force pushed an update:

Uses String.concat
Uses = instead of ==
Renames ?desc to ?userdesc

I didn't do anything wrt lazier logging or to round-robin the TCP DNS (which is hard to do here because apparently our secondary DNS server doesn't do TCP, but also because it is incompatible with the "bolted onto the side" way I've done things here)

This change looks good to me.


explicitly set log level of pydocstyle

Otherwise the pydocstyle logger uses an effective level of 1 flooding the output with debug messages.

Codecov Report

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

@@           Coverage Diff           @@
##           master     #126   +/-   ##
=======================================
  Coverage   78.35%   78.35%
=======================================
  Files          51       51
  Lines        2864     2864
  Branches      483      483
=======================================
  Hits         2244     2244
  Misses        594      594
  Partials       26       26

Continue to review full report at Codecov.

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


javascript code extension, beginscripts doesn't recognize in partials view

when using code extension to send the javascript from a partial view to the index, the code is not recognized, would this correction be possible? same works in visual studio 2019
in vs 2019

@ryanbrandenburg Is this a known issue?

We've had some issues about JavaScript not being represented properly but not this one specifically. I'd say send it our way.

Moved this issue to the dotnet/razor-tooling repo as https://github.com/dotnet/razor-tooling/issues/5962. Closing.


"Error occured in launcher loop"

After one of the latest updates, I've started to get these:
[2017-04-20 18:59:58][   ERROR][overseer] Error occured in launcher loop.
Traceback (most recent call last):
File "...\overseer.py", line 364, in launch
await self._launch(update_spawns)
File "...\overseer.py", line 396, in _launch
for point, (spawn_id, spawn_seconds) in spawns_iter:
TypeError: 'NoneType' object is not iterable
The scanner will always forcibly close after these. Maybe something to do with the database? I'm using MySQL and made a new database just to be sure, but the problem still exists.

Same thing is happening with me while using Postgres, thought I was the only one.

I removed the spawns.pickle file and now the problem seems to be gone.

I changed a few config settings around that had to do with spawns and altitude and what not, and it seems to be working for now, but I will try that next if it happens again. Thank you.

Same here. Same setup. Same solution.

Seems to happen again and again. Deleting spawns.pickle only solves it temporarily

I haven't had the problem anymore. I'm not familiar enough with the code to say for sure, but I guess it has something to do with unknown spawns that are out of reach (outside of map boundaries, e.g. when using polygons).  By removing the spawns.pickle file, those out-of-reach spawns are gone from the workers' mind, so they can concentrate on the spawns that matter. This will help (or at least in my case it helped) with skipping, timeouts and clustering too.
Anyway, I hope this will get some attention, as it's pretty critical.

Has anyone tried running scan.py --bootstrap instead of deleting spawns.pickle? I had a 'NoneType' object is not iterable crash yesterday, and that solved it. Not sure if it was the same place in the code as described in this bug report, though.

This crash has been happening to me a lot since the increased event spawns arrived. Running scan.py --bootstrap does indeed work around it without having to delete spawns.pickle.

I believe the cause is this line in spawns.py:add_known():
# add so that have_point() will be up to date
self.known[point] = None

I was getting this frequently up until a few weeks ago when I updated everything (requirements and monocole). The fix had always been to delete the pickle.


Update use-postman.md

Updating postman article on docs to link to samples repo postman collection

@EXPEkesheth
Can you review the proposed changes?
When the changes are ready for publication, add a #sign-off comment to signal that the PR is ready for the review team to merge.
#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

#sign-off


More detailed CONTRIBUTING.md

Adding more detailed and structured process to contributors' guide.

@rastala,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

Tweaked a bit and merged.


help menu error

I get an error everytime I start pfiefer.
Undefined function or variable 'setHelpMenus'.
Error in PFEIFER (line 54)
setHelpMenus(workbenchFigObj)      % and set help buttons
osx 10.12
matlab 2017b

pfiefer still seems to work fine otherwise

This is the most up to date version of PFEIFER?

You need to add the entire PFEIFER folder to the path, not just “source.”

On Feb 12, 2018, at 2:47 PM, wgood<EMAIL_ADDRESS>wrote:
This is the most up to date version of PFEIFER?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub https://github.com/SCIInstitute/PFEIFER/issues/32#issuecomment-365073358, or mute the thread https://github.com/notifications/unsubscribe-auth/AdF4h83NSg-9z2kawkvBhg2waeIee_YQks5tULFogaJpZM4SC3GA.

Sorry Jess. The language in the README and the manual did not agree with each other. Both documents now say that you have to add the entire PFEIFER folder to the matlab path.


k8s: Incorrect container count in pod-view

The metrics pod (from the service) has 5 containers, not one:

Report: report_wrong_pod_container_count.json.gz

metrics pod != monitoring pod.  Metrics report periodically dumps to BI, monitoring pod containers all the jazz:


We need to print progress when committing to a remote store

We've gotten to the point where just sending the bytes to the server is the slowest part of most writes. Yay. But now, the progress for e.g., csv-import, sync, and other things is not as useful because we get near 100% and then it pauses there for awhile during upload.

I'm gonna close this one. 1) Because the "commit" phase is quite a bit faster than it was when we wrote this and 2) because I think it's superseded by implementing batched writing which there is now a clear path to.


Links to Adobe DAM content seem to have domain overridden

Links to Adobe DAM content seem to be overridden
https://main--elixirsolutions--hlxsites.hlx.live/providers
My current assumption is this will resolve when we go live as it seems to be auto-replacing the domain.
https://www.elixirsolutions.com/content/dam/elixirdotcom/providers/Q2-Communication_061923.pdf
https://main--elixirsolutions--hlxsites.hlx.live/content/dam/elixirdotcom/providers/Q2-Communication_061923.pdf

yea, we relativize links so once we go live it will be resolved, but we can change the logic slightly so that it ignores certain paths or something along those lines.

Ok cool that works for me. I don't anticipate any problems currently just wanted to make sure it is expected.


PHOENIX-2890 Extend IndexTool to allow incremental index rebuilds

Thank you so much @chrajeshbabu for the review, Raised PHOENIX-3525 for the same.
@JamesRTaylor  ,do you want to take a look or it's ok to commit now?

Already merged.


Feature request: Ability to dismiss goblins

Hello! Could we please have some simple way to dismiss a goblin if we don't want to trade with them right now, aside from killing them, which I'd really rather not do?

Goblins will only stop following once you buy something. It's possible I can add a config option but that is essentially turning off their personality. Or you can just dig a hole and push them in it. Let me know.


Add dataset function for each dataset

🚀 Feature
For each dataset we can also add a dataset function that returns a train,valid dataset with default transforms included.
This would also reduce the amount of repeated code on icevision tutorials.

Added dataset function for fridge

I can get started on coco

I can do it on pets


border on iron-input

Is there a way to override the styling of the iron-input in the paper-search-bar so that it has a 1px border around it? I would like to have the border around the iron-input and not the input inside it. I would like for it to look like this.

@ratracegrad, I don't think you can today but it sounds like a good idea. Feel free to provide a PR that adds a mixin to the iron-input.


Formatting fixes for "Manage Trigger Security"

This fixes #5007

Changed formatting of user name "JohnDoe" in first and second paragraphs to bold instead of inline-code to be consistent with rest of formatting in first paragraph.
Fixed example code formatting to be in a fenced code block instead of each line being inline-code.
Fixed exploit example code to be realistic and work outside of the [master] database.
Removed extraneous space(s) at the end of most lines within the fenced code blocks (fenced code blocks preserve whitespace and hence extra spaces at the end of lines provides no markdown rendering benefit, but they do make it messier when readers copy and paste to run it locally).
Removed extraneous space at the end of most lines, just before the semicolon (just looks like poor formatting).
Changed UNION to UNION ALL in second fenced code block (best practice).
Made all fenced code blocks use "sql" style whereas previously only one of them used it and the rest did not have a style. I could not find any consistency across several other pages that I just checked for reference, so I erred on the side of readability (and what I, as a reader, prefer).
Added uppercase-N prefix to string literals in final fenced code block (query to disable all DML triggers) as the variable is NVARCHAR and MS documentation should show best practices.
Added link to "Logon Triggers" in "See Also" section.

@srutzky : Thanks for your contribution! The author(s) have been notified to review your proposed change.

@rothja -- Jason, please review these changes.

@srutzky This was a lot of work making the article better. Thank you! I want to get one other person to verify the changes before we publish. @pmasl Could you please take look at the changes proposed here to the "Manage Trigger Security" article. See the file changes in this PR. Note that Solomon has graciously offered to suggest the changes to address another customer issue here: .#5007. #assign:pmasl. Pedro, if you think someone else should review, please let me know. Thank you.

pmasl. is not a valid GitHub ID, or is not a collaborator on this repo.

Note: I'm contacting Pedro offline. Thanks!

@srutzky These changes have been reviewed/approved. I will look at the merge conflicts to resolve and sign off.

@rothja : Thanks for your contribution! The author(s) have been notified to review your proposed change.

#sign-off

@rothja  Do you want me to fix something?

@srutzky No. We changed how we do the "Applies to" at the top, so it caused a merge conflict. I fixed it for you and pushed the changes to your branch. I hope that is OK. I merged these changes in along with my conflict fix. Thank you!

@rothja Yes, that's perfect. I see now the conflict you were talking about. Looks good.


About Log Message Formatting on Reconnect

Hey Aymeric,
I noticed that by this line
https://github.com/python-websockets/websockets/blob/98f236f89529d317628fe8ee3d4d0564e675f68d/src/websockets/asyncio/client.py#L525
the full traceback of the current Exception is included in the log message, which looks confusing to me.

Example
[2024-09-13 11:18:03] [INFO] ! connect failed; reconnecting in 0.8 seconds
Traceback (most recent call last):
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 411, in __aiter__
    async with self as protocol:
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 395, in __aenter__
    return await self
           ^^^^^^^^^^
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 376, in __await_impl__
    _transport, self.connection = await self.create_connection()
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1129, in create_connection
    raise OSError('Multiple exceptions: {}'.format(
OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 8001, 0, 0), [Errno 111] Connect call failed ('<IP_ADDRESS>', 8001)
[2024-09-13 11:18:04] [INFO] ! connect failed; reconnecting in 3.1 seconds
Traceback (most recent call last):
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 411, in __aiter__
    async with self as protocol:
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 395, in __aenter__
    return await self
           ^^^^^^^^^^
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 376, in __await_impl__
    _transport, self.connection = await self.create_connection()
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1129, in create_connection
    raise OSError('Multiple exceptions: {}'.format(
OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 8001, 0, 0), [Errno 111] Connect call failed ('<IP_ADDRESS>', 8001)
[2024-09-13 11:18:07] [INFO] ! connect failed; reconnecting in 5.0 seconds
Traceback (most recent call last):
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 411, in __aiter__
    async with self as protocol:
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 395, in __aenter__
    return await self
           ^^^^^^^^^^
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 376, in __await_impl__
    _transport, self.connection = await self.create_connection()
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1129, in create_connection
    raise OSError('Multiple exceptions: {}'.format(
OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 8001, 0, 0), [Errno 111] Connect call failed ('<IP_ADDRESS>', 8001)
[2024-09-13 11:18:12] [INFO] ! connect failed; reconnecting in 8.1 seconds
Traceback (most recent call last):
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 411, in __aiter__
    async with self as protocol:
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 395, in __aenter__
    return await self
           ^^^^^^^^^^
  File "/▉▉▉/.venv/lib/python3.12/site-packages/websockets/asyncio/client.py", line 376, in __await_impl__
    _transport, self.connection = await self.create_connection()
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/asyncio/base_events.py", line 1129, in create_connection
    raise OSError('Multiple exceptions: {}'.format(
OSError: Multiple exceptions: [Errno 111] Connect call failed ('::1', 8001, 0, 0), [Errno 111] Connect call failed ('<IP_ADDRESS>', 8001)

Since this exception is indeed useful to get to know about failed connections or received responses, how do feel about just deleting the traceback and keeping the exception message with one of these options?

My opinion is mainly "let's not get too fancy, or else someone is going to complain that it doesn't work in their APM / ..."

I've never seen the pattern of removing the traceback in 18 years of building software in Python so I'm worried that it may have consequences that we would regret.

Okay, so how about turning
https://github.com/python-websockets/websockets/blob/76f6f573e2ecb279230c2bf56c07bf4d4f717147/src/websockets/asyncio/client.py#L523-L527
into
self.logger.info(
    "! connect failed; reconnecting in %.1f seconds",
    delay,
)
self.logger.debug(exc, exc_info=True)  # or similar

With that, we would have a middle way: The traceback is still there if desired, but does not irritate the average user right away. As the traceback is also only useful for developers, the DEBUG level would be a good fit in my opinion.

I looked into this over the weekend and I've grown sympathetic to your position.
I started standardizing on "all DEBUG and ERROR logs have tracebacks; none of the INFO and WARNING logs do".
Rationale:

you want everything in DEBUG
if it's an ERROR, it's bad enough that you want the traceback
if it's INFO or WARNING, it's just a timestamp and the info that something normal or abnormal (respectively) happened, but you can ignore it and you'll be OK

The double log is an interesting idea. To be honest I was planning to keep it simple; if it crashes, just connect (instead of using the magic reconnect) and see the error you get.

I started standardizing on "all DEBUG and ERROR logs have tracebacks; none of the INFO and WARNING logs do".

Ah, okay, so my enhancement request would be covered by this. I like :+1:

To be honest I was planning to keep it simple; if it crashes, just connect (instead of using the magic reconnect) and see the error you get.

Sorry, I am unsure what you mean by that.

I meant "if I do simple INFO logging without traceback and a user wants the traceback, they can get the traceback by calling await connect(...) instead of async for ... in connect (...) — then the error will be raised.

I took a quick stab at fixing it. Leaving it there for tonight; I'll think about it more later. Meanwhile, happy to get your feedback.


Fix test: parsing::test_unknown_explicit_vr_is_error

Background
The test parsing::test_unknown_explicit_vr_is_error was originally written to verify that an unrecognized VR produced a specific error result. However producing an error in this case prevents the rest of the DICOM stream from being read. Since the VR is only an indicator as to how the element's value should be decoded this was changed to allow dicom elements to continue being parsed from the stream and the user of the library can decide how to proceed with the element. The DicomElement uses a sentinel VR value of INVALID when an unknown VR is parsed. The test for this case was never updated however, and is currently failing as it expects an error.
Changes
Update the parsing::test_unknown_explicit_vr_is_error test to instead not expect an errror by validate that the VR comes back as INVALID, and also verify that at least one subsequent element can be successfully parsed from the stream.

A series of commits/changes were made while investigating this, finalizing in the test being updated to pass in 256871d3a68e4f7af31a31d1af9571e0affa577d


Indicator setup > Targets > Make numeric fields and error handling more consistent

This ticket was peeled from a comment on #1214.
Acceptance criteria
Baseline

[ ] 1. I cannot type any special characters, including a negative sign.

[x] 2. If I leave the baseline field empty or somehow manage to enter an invalid value, the error message says: Please enter a number greater than or equal to zero.

Targets (all frequencies including LoP only and non-time-aware)

[ ] 1. I cannot type any special characters, including a negative sign.

[ ] 2. If I leave a target value field empty or somehow manage to enter an invalid value, the error message says: Please enter a number greater than zero.

All error messages on this tab
Please make sure the error message styling is consistently the smaller size text in the same shade of red. In the screenshot below, the error message under the targets table looks good but the one under baseline looks too large and is possibly a different shade of red. (Hard to tell because of the different backgrounds.)

@camgnostic I am able to type a + sign in the Baseline and Target value fields. I should not be able to do this, just as I can't type a - sign. (Sorry if the criteria was unclear -- I was using "-" as an example of a common symbol people might try to type, didn't mean to suggest that "+" is acceptable.)

Not sure if this is related to work on this ticket, but it appears to be a Goat Tree regression: I am now seeing type-ahead results in the Unit of Measure field; it's trying to fill in my address.
Verified that this behavior does not occur on Prod. Just let me know if it should be a separate ticket.

Life of Program target frequency: I am no longer getting an error message when I leave the LoP target value field empty.
Here's what happens on Prod:

What I was hoping for with this ticket is that the "Please complete all fields" warnings at the top would NOT be affected, just that we'd add the consistent messaging underneath the target table: Please enter a number greater than zero.

Midline/Endline: Behaves as expected in terms of warning messages at the top and under the target table, but the text was not updated to: Please enter a number greater than zero.

Issues with auto-suggestions and +- symbols resolved.


rpc-core

https://github.com/0xSpaceShard/starknet-devnet-rs/blob/337b24fbab448db993c93239bba762d13d97208c/Cargo.toml#L31
Let's keep original name of the package because its not quite clear when you see rpc-core what it is, there's no such crate so one has to go and find that it's actually foundry

@marioiordanov this will be closed with the crate PR


Run test pods with readOnlyRootFileSystem: true

Up until now the test pods were being spawned with writable root
filesystem (readOnlyRootFilesystem: false). This is against the best
security practices as setting the readOnlyRootFileSystem to false
increases the size of the attack surface.
This patch ensures that each pod spawned by the test-operator has the
readOnlyRootFilesystem to true by default. It is possible to run a
pod with writable file system by setting privileged: true.
Depends-On: https://github.com/openstack-k8s-operators/tcib/pull/233

recheck

recheck

recheck

/test test-operator-build-deploy

The promotion for the podified-antelope-centos9 did not pass yesterday (tcib image does not contain the fix yet):

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a7e6aed5aea04e80b3c36d5a39b31db1

-> We have to wait.


select date_format error

The query "SELECT date_format(mydate, 'yyyy-mm-dd') AS date, COUNT(*) AS count, SUM('aaa') AS sum FROM table GROUP BY date_format(mydate, 'yyyy-mm-dd')" got error w/ JDBC, but works w/o JDBC.
The error, com.amazon.opendistroforelasticsearch.jdbc.protocol.exceptions.InternalServerErrorException.

After referenced PR the query doesn't fails anymore
POST _opendistro/_sql?format=jdbc
{
  "query": "SELECT date_format(utc_time, 'dd-MM-YYYY'), count(*)  from kibana_sample_data_logs group by date_format(utc_time, 'dd-MM-YYYY')"
}

but has suspicious column in the result.

This is SQL plugin issue, not JDBC. It will be worket here:
sql-111


Add PowerShell example to install a Windows service

Summary
Add a PowerShell example to the How to: Install and Uninstall Services page.
Fixes #5460

No worries at all @rpetrusha
I think you were all quite occupied preparing the dotnet conf, with dotnet core 3.0 ;)


[web] PathUrlStrategy doesn't work with nested paths

Steps to Reproduce
Channel master/stable
Set PathUrlStrategy on default project
Run locally and try to navigate to /a/b
Error: The script has an unsupported MIME type ('text/html').
But if you change entry point script in index.html from

to

then it works as expected
problem with:
class FlutterTrustedTypesPolicy {
    /**
     * Constructs the policy.
     * @param {[RegExp]} validPatterns the patterns to test URLs
     * @param {String} policyName the policy name (optional)
     */
    constructor(validPatterns, policyName = "flutter-js") {
      const patterns = validPatterns || [
        /\.dart\.js$/,
        /^flutter_service_worker.js$/
      ];
      if (window.trustedTypes) {
        this.policy = trustedTypes.createPolicy(policyName, {
          createScriptURL: function(url) {
            const parsed = new URL(url, window.location);
            const file = parsed.pathname.split("/").pop();
            const matches = patterns.some((pattern) => pattern.test(file));
            if (matches) {
              return parsed.toString();
            }
            console.error(
              "URL rejected by TrustedTypes policy",
              policyName, ":", url, "(download prevented)");
          }
        });
      }
    }
  }

fix:             const parsed = new URL(url, document.baseURI);

@Gorniv
This seems to have been already fixed just yesterday :) and should be available on latest master. So try to switch to it and try your case again to see it working.

Fixed in master! Sorry)


Add ONNX support

maybe the two convert-to-onnx.ipynb files are duplicate (one in / and the other one in /misc)?

@deep-diver addressed your comments.


sync to live

OPS Build status updates of commit 5a2e796:
:clock10: Preparing: average preparing time is 42 sec(s)

OPS Build status updates of commit 5a2e796:
:x: Validation status: errors
Please follow instructions here which may help to resolve issue.

File
Status
Preview URL
Details

:x:Error

Details

[Error] Cannot sync git repo to specified state. Please make sure the pull request is mergeable. Details: Git merge results in conflict, repository remote url: https://github.com/MicrosoftDocs/windows-iotcore-docs, working directory: W:\zzpb-s, target branch: live, target branch commit: 3db243e81e0ff79e3a681f7b6bf0554fb4ab4f65, source branch: master, source branch commit: .

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

OPS Build status updates of commit 4c5b6c6:
:clock10: Preparing: average preparing time is 42 sec(s)

OPS Build status updates of commit 4c5b6c6:
:clock10: Incremental building: average incremental building time is 18 sec(s)

OPS Build status updates of commit 4c5b6c6:
:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

windows-iotcore/release-notes/Insider/RPi3BP.md
:white_check_mark:Succeeded

For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.


scrollbar doesn't scroll all the way when thumb hits sliderminthumbsize

e.g., sliceview in examples/view dialog with 2880 can only scroll to 2862..

added ThSizeReal that reflects real size of thumb -- and added SliderMoved signal emit which was missing before.


Failing testcase for #7140

See https://github.com/doctrine/doctrine2/issues/7140
2nd level relation hydration fails when object is already loaded and is then "reloaded" by a separate query where that relation is joined in.

This looks similar to https://github.com/doctrine/doctrine2/pull/625 in nature.

Looks to be the same problem yeah. Nothing to be done about it until 3.0 then? Making the test case at least gave us some potential workarounds to use for now.

It's not that nothing can be done, but rather that a decision about eager loading has to be taken, and it might result in a BC break.
We didn't find a solution on our end (yet).


How to fix the camera perspective

NOTE: Any issue that does not follow the below template will be immediately closed and not re-opened until the template structure is adhered to.

Precheck

Do not use the issues tracker for help or support (try VRTK Slack channel, Stack Overflow, etc.)
For proposing a new feature, please check existing open and closed issues before creating a duplicate
For bugs, do a quick search and make sure the bug has not yet been reported
Finally, be excellent to each other, and party on dudes!

Environment

Source of VRTK (Unity Asset Store or Github)
Version of VRTK (Unity Asset Store/Github release number) (Github master commit hash)
Version of Unity3D (e.g. Unity 5.4.4f1)
Hardware used (e.g. Vive/Oculus)
SDK used (e.g. OpenVR/SteamVR/Oculus Utilities)

Steps to reproduce
Attempt to recreate the issue in a VRTK example scene and provide steps to reproduce in a clear text step by step list. Include code samples, errors and stacktraces if appropriate.
Expected behavior
Briefly describe what you expect should happen
Current behavior
Briefly describe what is actually happening

I think you forgot to actually tell us about your issue. There is nothing here except the issue's title which doesn't offer any info.


remove unused line-reader

line-reader is not in use, forgot to remove the require.

merge into nb-NO also