Enhancement: Sharing whole project instead of current file

Description
Current implementation only shares current project. It would be more useful if I could share the whole project so two persons can work on different files. Ideally jumping to the file by clicking an avatar.
Is anything like this planned for the future?

Had a similar idea, something like "code conference": basically, allows you to share a read-only workspace to others. Would like to know if this is practical. Thanks.

I was not only thinking about a "read-only" workspace, but also the possibility to edit other files.
But maybe more fine-grained control would be interesting like: Read/Write; One File/Multiple Files/Whole project.

👍

A colleague and I tested it, and it was the first thing we mentioned as "needed to be useful", but we also agreed that it would be enough to "only" share the open tabs. I mean, I have the same code base on my computer, so i can use that to find out which extra file is needed, and I can ask him to open it. In case security might be a big issue against it. ;)

@dennisvandehoef Good point. But it wouldn't be the case if the workspace is read-only. The tabs being changed should be distinguishable anyways, but at the same time you should be able to explore other files if you want. This is the desirable setting for a "code conference" use case.

@dennisvandehoef Thats why it would be good to have finer control. So you can set up read-only or write only to certain files. So you can choose your own security level.
For example a use case for browsing the whole project would be helping someone with his project, or looking for some bugs, so you down have to setup the code yourself. Would make it easier than setting up a Codepen every time.

Thanks for taking the time to share this idea! :bow: Repository-wide sharing and/or multi-file sharing is indeed something we'd like to provide in the future.
As we prioritize future work, we'll be looking at feedback issues like this one to determine where we can deliver the most benefit with the available engineering resources (i.e., full-time Atom engineers, community maintainers, and new contributors). In the meantime, if this is a feature that would bring you joy in your daily workflow, adding a :+1: emoji reaction to the issue body is a great way to let us know.

:wave: Good news for people interested in collaborating on multiple files simultaneously: Teletype v0.3.0 adds preliminary support for hosts to share multiple simultaneous files with guests. Guests are free to navigate among the files shared by the host, and guests can edit those files independently of the file that the host is currently viewing/editing.
(demo gif)
This update is the first step toward implementing RFC-001. Note that RFC-001 focuses on the host sharing their workspace, which is a bit different than the host sharing an entire project. (See Rational and alternatives in RFC-001). We're interested in Teletype allowing a host to share an entire project at some point in the future. In the meantime though, we anticipate that sharing a workspace will address many of the use cases that developers have in mind when wanting to share an entire project.
If you're interested in tracking the progress on RFC-001, feel free to subscribe to the tracking issue for updates. 👌

@jasonrudolph A better version of this would be to see the whole file structure as explained in https://github.com/atom/teletype/issues/227
The current version (0.9.0) allows ppl to work on multiple files, but it is very hard to navigate, especially when the host have files opened from different projects at once. What makes it even harder in the current state is that the clients will have the host name on each tab displayed as "@HostName SuperlongFilename.js" witch is cut down to "@HostName Su" and then have no clue what file that tab represents until opening it.

Hi. After testing Teletype a bit and looking into this issue/reading the above comments, here's my 2 squid:
The most important way for me to be able to share is definitely to share the PROJECT FOLDER in its entirety. There is little utility in having to ping someone to nav over to a file so you can see it again, while hoping they never timeout. While sharing many files over Telemetry, any problem means I need to go click a bunch of tabs in a row.
So the best way to handle security/selective sharing in this case it seems would be to use something like a GITIGNORE, and just list any patterns or filters for who you'd like to not see what.

@AGkitchen
Before a full permission system (like GITIGNORE) is setup, it makes sense to me to that a simple stepping stone would be to simply allow the guest to see the project tree, and request that files should be opened.
Later a role system could be implemented on top of this base functionality.

Been using teletype extensively at work with other devs on a new project, and while it's fantastic, the thing we'd really need in order to be even nearly as productive as in-person pairing is having access to the whole project. We're working on golang projects, and the ability to quickly jump between packages is ridiculously important!

please, make it

Is there any update? I think this is very important!

Being able to share en entire project is very important AND dangerous. When the "share" button is presses, the user should be prompt whether he wants to share current file or the entire project.
This means that git only becomes a way to have a backup as all the work is done using P2P!
This would simply be amazing.
Any update on this by the way?

Will there be a function to permanently share the file to editors, similar to the way Google Drive works? This way, the host does not have to be online for guests to edit. Is there another way to do this?
Thanks!

Unfortunately this has myself and my team using VSCode + VS Live Share as we pair quite heavily. I miss atom :(. Do you need help contributing to this feature? Is there a way we can be given some direction or thoughts as to the route that might be taken?

Will there be a function to permanently share the file to editors, similar to the way Google Drive works? This way, the host does not have to be online for guests to edit. Is there another way to do this?
Thanks!

I would like this. I think possibly having a server version of Teletype would be a good implementation. Possibly have a backend of SFTP, have some handshake to connect to a project on a server, and have all the regular teletype features from there.
Me and a friend are working on a project, but we have to push to Git when we're dealing with programming flaws. Personally, I don't feel like Git should be used this way with many, many commits, but maybe I'm wrong! It would be nice to share an entire folder (like Google Drive), and be able to simultaneously edit the project.
Interesting idea to keep in mind...

It would be nice to share an entire folder (like Google Drive), and be able to simultaneously edit the project.

Is it on the roadmap to have the project view on the sidebar as well, or just the ability for the host to open files and the guest to see only the opened files in new tabs? I can see his code when he opens it, but my project view is blank. Would be helpful to have access to all files in the project, with some sort of host approval of course.
cc @jmwoloso

The project seems almost abandoned. You can take a look at VSCode's Liveshare which works really well.
Moreover (and sadly), Atom generally almost doesn't receive any new update while VSCode does.

Dang - not a fan of VSCode but will give it a shot. Thanks!

Dang - not a fan of VSCode but will give it a shot. Thanks!

I was thinking the same thing at the beginning too! But you can install the Atom keybindings extension on VSCode for example, and there is way more extensions (and more maintained!) as well!
I strongly advise anyone reading this to give VSCode a try as Atom stays in its current state (not gaining any new features).

Hey @pxlrbt is there any reason why you closed this issue? If you're getting annoyed about notifications on the thread you can unsubscribe from the issue itself, instead of closing it...

Yes this issue is still unresolved! Why are you closing it?

Sorry it seemed as this won't be implemented. But you're right. Unsubscribing is the better solution.


With the right buffer disabled on vimEnter, text is cut/hidden.

This is my NoNeckPain config:
return {
	"shortcuts/no-neck-pain.nvim",
	config = function()
		require("no-neck-pain").setup({
			autocmds = {
				enableOnVimEnter = true,
				-- enableOnVimEnter = false,
			},
			buffers = {
				right = {
					enabled = false,
				},
			},
		})
	end,
}

When using NeoVide, this is what I see when I open a file using neovide (neovide telescope.lua in terminal):

Notice that the text is hidden/cut. If I do something on the file, however, the problem fixes itself. For example, if I go to the bottom of the file by doing G, the text is shown.
Interestingly, I cannot reproduce this on the terminal because my config doesnt make NoNeckTree auto-open there.

Hey @vinoff, thanks for reporting and using the plugin!
I guess there might be a conflict with Neovide in that case because I have this exact same setup and can't reproduce! I'll take a look ASAP

Actually I just realized, it's not cut or hidden, you have the right buffer enabled, no?
You can try to add buffers={right={enabled=false}} at the root of your config


v0.1.1

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

7 of 7 (100.0%)  changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.3%) to 44.061%

Totals

Change from base Build<PHONE_NUMBER>:
1.3%

Covered Lines:
115

Relevant Lines:
261

💛  - Coveralls


adding some information

im create for newbie to understanding about runingn Taiko node

We still prefer using https://taiko.xyz/docs/guides/run-a-taiko-node

thank for answer,  keep on my repo


Merge master into frigate-v0.12

Hi, I'm using this integration from the frigate-v0.12 branch directly as I'm using Frigate 0.12 beta.
However, I think master had some fixes regarding newest versions of Home Assistant that frigate-v0.12 still lacks.
Would it be possible to merge master into frigate-v0.12 so I can get the latest improvements?
(there are merge conflicts)

The 0.12 branch already has those fixes https://github.com/blakeblackshear/frigate-hass-integration/commit/8b80b6b414ed6c366edb0b972385130be8974b28

Interesting. Thank you. The reason why I asked was:

It's all minor build/doc related things: https://github.com/blakeblackshear/frigate-hass-integration/compare/frigate-v0.12...master
I was imagining we'd just waiting until v0.12 is out on the backend end, then just merge frigate-v0.12 here back in master and be done.


Track editing : Undo Redo

Provides undo redo on the track while editing.

https://github.com/Soreine/immutable-undo

Implemented for region moves. More actions soon. We used https://github.com/ArthurClemens/Javascript-Undo-Manager


Fake error message "Error creating cluster"

When I tried to create a k8s cluster using the wizard:

I got the following error at the end:

But later I tried to see whether the new cluster is created using:

I successfully configured the cluster to my local registry. And I can operate on the nodes:

Not sure what the error message was about but seems like it should not be there.

That's weird.  It's like we're picking up a progress message as an error message.  Not sure how to reproduce this one - does it happen consistently for you?

Yes, tried to create another cluster and the error persists:

But later the cluster can be configured successfully:

Ok, here is some further info:
Region: Central US
Node type: Standard D2 v2
Node count: 2
Kubernetes version: 1.7.9
Azure CLI Version: 2.0.28
I also noticed some errors in activity log:
{
    "authorization": {
        "action": "Microsoft.ContainerService/managedClusters/write",
        "scope": "/subscriptions/ffa52f27-be12-4cad-b1ea-c2c241b6cceb/resourceGroups/romlk8s/providers/Microsoft.ContainerService/managedClusters/romlk8s"
    },
    "caller"<EMAIL_ADDRESS>    "channels": "Operation",
    "claims": {...
    },
    "correlationId": "2d988027-b82f-41de-98cb-33427f94cb26",
    "eventDataId": "c2914a36-5dd7-4bc9-885e-bcbf84350a75",
    "eventName": {
        "value": "EndRequest",
        "localizedValue": "End request"
    },
    "category": {
        "value": "Administrative",
        "localizedValue": "Administrative"
    },
    "eventTimestamp": "2018-03-08T03:14:46.6334217Z",
    "id": "/subscriptions/ffa52f27-be12-4cad-b1ea-c2c241b6cceb/resourceGroups/romlk8s/providers/Microsoft.ContainerService/managedClusters/romlk8s/events/c2914a36-5dd7-4bc9-885e-bcbf84350a75/ticks/636560756866334217",
    "level": "Error",
    "operationId": "2d988027-b82f-41de-98cb-33427f94cb26",
    "operationName": {
        "value": "Microsoft.ContainerService/managedClusters/write",
        "localizedValue": "Microsoft.ContainerService/managedClusters/write"
    },
    "resourceGroupName": "romlk8s",
    "resourceProviderName": {
        "value": "Microsoft.ContainerService",
        "localizedValue": "Microsoft.ContainerService"
    },
    "resourceType": {
        "value": "Microsoft.ContainerService/managedClusters",
        "localizedValue": "Microsoft.ContainerService/managedClusters"
    },
    "resourceId": "/subscriptions/ffa52f27-be12-4cad-b1ea-c2c241b6cceb/resourceGroups/romlk8s/providers/Microsoft.ContainerService/managedClusters/romlk8s",
    "status": {
        "value": "Failed",
        "localizedValue": "Failed"
    },
    "subStatus": {
        "value": "BadRequest",
        "localizedValue": "Bad Request (HTTP Status Code: 400)"
    },
    "submissionTimestamp": "2018-03-08T03:15:00.1113487Z",
    "subscriptionId": "ffa52f27-be12-4cad-b1ea-c2c241b6cceb",
    "properties": {
        "statusCode": "BadRequest",
        "serviceRequestId": "96dd9cbd-8096-4f68-b242-a6b7be91adc8",
        "statusMessage": "{\"code\":\"ServicePrincipalNotFound\",\"message\":\"Service principal clientID: f50c07ad-0178-4670-a523-deecf6fbaaad not found in Active Directory tenant 72f988bf-86f1-41af-91ab-2d7cd011db47, Please see https://aka.ms/acs-sp-help for more details.\"}"
    },
    "relatedEvents": []
}

I wonder if the Azure CLI must be writing to standard error even when it succeeds.  (We rely on checking stderr because the exit code is not reliable.)  I've seen this very intermittently on Windows; perhaps it happens more frequently on Mac.  Or perhaps the CLI is prematurely reporting an error but the operation has already been successfully kicked off in ARM.
If you do the corresponding az aks create from the command line, what do you see?

I've noticed I often see "Retrying role assignment creation" during some of these operations on the CLI.  I wonder if that's being printed to stderr and that's throwing us off (because although it had to retry, the retry eventually succeeded).  That would be consistent with the SP-related message you see, where perhaps this is trying to overwrite the retry with a progress message... waves hands handwavily

I also met this issue. And no such a problem when creating an AKS cluster in Azure CLI.


WIP xDSL notebook

Adds a jupyter notebook with some prototype code to translate a devito node into an xdsl structure.

Should have been a PR in the cloned repository


Multilingual preprocessing

For this multilingual model (en-ROMANCE), test set
I am having trouble running inputs with language codes through marian.
Specifically, if I add the codes after running spm_encode, things are fine.
If I try to use the preprocessing logic in preprocess.sh, things break.
test set: >>pt<< Don't spend so much time watching TV.
Using preprocess.sh, I get either
▁> > pt < < ▁Don ' t ▁spend ▁so ▁much ▁time ▁watching ▁TV .

or
>>pt<< ▁> > pt < < ▁Don ' t ▁spend ▁so ▁much ▁time ▁watching ▁TV .

but both of those cause marian_decoder to error.
The correct input seems to be
>>pt<< ▁Don ' t ▁spend ▁so ▁much ▁time ▁watching ▁TV .

With the code added after spm_encode is run. Is that correct?

Yes, the last line is correct. You should preprocess.sh without the language flag. That will be added after tokenisation - so yes, it should come after spm-encode. preprocess.sh should work if you have plain text as input without the language token. Does that work?

Yes, thanks!


Add support for Windows (Fixes #28)

Primarily this:

Adds Windows support to copypasta
Adds rusttype as a font rendering backend
Creates an interface to abstract over how we interact with a TTY-like thing (EventedRW)
Adds safe winpty bindings
Adds winpty as a TTY shim
Adds appveyor CI support

The windows support is not perfect yet but it's good enough that I think it makes sense to merge it. Any unresolved issues are listed here.
Apologies for the unrelated formatting changes, I can dig through and remove them from the pull but it might take me a little while.

Thanks a ton for working on this! Great to see a PR being opened for this. :)
It will probably take a while until this is merged because I'm pretty sure the focus is on scrollback right now, so feel free to keep working on the remaining issues in this PR. But it looks like none of these should be blockers? However any issue remains before this is merged, the issues should be transferred to this repository.

Yeah they were mostly merge issues, I haven't quite mastered git yet :wink:.
I forgot to mention in the original post but it'd be good if someone could take a look at the rust build script for winpty because I don't want to create a pull request for a foreign language without it getting some review first.

@zacps this is incredibly exciting for me to see, and I know there is a good number of people wanting to use Alacritty on windows but just waiting for support to land. Thank you very much for not only working on this but actually delivering a fully functional PR!
I'm looking forward to taking this for a spin and adding a review. My inclination here is to pull this in without too much delay since it's largely new work that won't impact existing platforms. Only caveat there is that I won't be able to make time to do so until June 29 at the earliest.
On a quick glance through the files, I noticed a dll and a couple of exes were added to the tree. Is it possible we could provide these through a build step instead of adding to the git tree?
One final note, I would very much like to invite you to join the project as a collaborator as our resident Windows expert, if you are open to it.
Thanks again!

So the dll is just a 32bit libclang required for 32 bit appveyor builds (bindgen dep). I'll try change the appveyor config to pull it down during the build.
The exe is unfortunately a bit different. Winpty is both an exe (that does the actual work) and a library to communicate with the exe. The library and bindings are included in the build step. The problem is that cargo has no way to 'publish' artifacts it doesn't understand. So while we can build the exe in the dependency there's no way to make it accessible to alacritty. The built file will be located in one of the target directories 'somewhere'.
Now that I think about it we could traverse the target directory and look for the exe with the latest timestamp, I'm not sure if that'd be worth doing.
I think I'll create an issue on the cargo repo so they're aware of this use case.
I'd love to join as a collaborator :smiley:

@zacps @jwilm, perhaps those binary dependencies should be packaged via the make step?
Storing an executable dependency with a binary shared object in Git strikes me as a particularly egregious antipattern.

binary132 It's definitely not ideal.
As I said in the previous comment the main issue is that cargo doesn't have any way to 'push' an artifact it doesn't understand up the dependency tree.
winpty-sys <- Binary dep built here
    winpty
        alacritty <- Binary needed here

Alacritty doesn't know where the built artifacts ended up. We can potentially try to search from them in target but that depends on the structure of target being stable (I don't know if it is).
The ideal solution is for a build script to be able to indicate that it wants to move an artifact into it's parent's target directory.

I also played around trying to implement finding the Binary and I realized we'd have to add gyp as another manually installed dependency.

You probably have already heard about the new ConPTY, but I'm adding a note here anyway. ConPTY is probably a saner way to implement this on Windows, but it will only be supported starting from the version of Windows 10 that's released in winter/fall 2018.

Would be good if we could have just one config file instead of alacritty_windows.yml. Then all I would need is the following config in my alacritty.yml file.
os:
    windows:
         shell:
              program: cmd

Anywhere folks can jump in to help make this happen?

I think we're just waiting on review from @jwilm at this point.

This is next on my list to review. One question I have is whether there's a way we can provide winpty.exe without committing a big binary to git, perhaps through a build process. Would love some ideas/feedback on this. I know @zacps has already spent some time investigating, but maybe you have some other ideas @jonathandturner? There's some discussion above.

I don't, sorry. Short of, say, fetching it with a build.rs or somesuch.

Would you prefer adding gyp as a build dependency and doing some path-hacking over including prebuilt binaries? Those are the only two options as far as I can tell.

@zacps I had another thought that we could maybe leverage git-lfs. Only problem is that GitHub charges an insane amount for that, and I would want to look into a self-hosted solution first.
For the gyp path, how do you estimate the complexity of that? If we don't have gyp, how hard would it be to regenerate these artifacts when the upstream changes?

Er, maybe git-lfs isn't that badly priced. I can't tell for certain, but it looks like $5 for 50 GB, which isn't too horrible.. might want to start shipping binaries before going down that path, though.

Don’t forget gh also charges for bandwidth and forks don’t have seamless
access I don’t believe. IMO gyp and things of that nature are stable enough
to be non versioned and put in like an s3 bucket or the like
On Mon, Sep 17, 2018 at 8:07 PM Joe Wilm<EMAIL_ADDRESS>wrote:

Er, maybe git-lfs isn't that badly priced. I can't tell for certain, but
it looks like $5 for 50 GB, which isn't too horrible.. might want to start
shipping binaries before going down that path, though.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jwilm/alacritty/pull/1374#issuecomment-422210469, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADDFmdGwDk_CwbynXHLRqVpiEsKCfPoQks5ucDlPgaJpZM4UniPy
.

Could download the github file from download release or from nuget server which is npm for .NET. https://www.nuget.org/packages/winpty.NET/
Here is a download link: https://www.nuget.org/api/v2/package/winpty.NET/0.4.2
nupkg is just a zip file so should be easy to extract it.
Would still be good to check the hash before extracting.

@prabirshrestha Huh I didn't know that existed, it includes the agent executable as well.
I think it probably makes sense to include this in Alacritty's build script rather than the winpty bindings.
I'll add that and try rebase it today/tomorrow.

Hi @zacps, really excited to see this work on Alacritty for Windows !
Are you using Windows Subsystem for Linux inside Alacritty ?
How could I build and try the Alacritty's version from your PR ? I read the README you updated, but did not notice any instructions, nor Makefile changes windows-specific (and I am not very experienced with Windows development).
Thanks a lot for your work !

@jwilm Do you have any ETA on when you can take a look at this?
It would be great to get this reviewed soon, following master is not no work.

@zacps while waiting for review, do you think it's worthy to provide chocolatey package for alacritty ?
Hopefully install it with choco install alacritty command.

@joelhandwell It's not something I'll do myself, but you're welcome to if you want.

@zacps I'm starting to run this branch locally on Linux just to check for any regressions. One thing I noticed initially is that scrolling-in-region bench seems to be slightly slower than master.
I'll try and do a review tomorrow morning after playing with it today. Maybe @chrisduerr can also take a look at the code. Note that I'm going to attempt to avoid a lot of nit-picking in the interests of getting this in-tree sooner than later.
Thanks for making the changes to move the binary artifacts out of git.

Regarding perf, I was testing with latest changes from master. Whatever the difference is with scrolling performance, it is real.

Are you using vtebench to test performance? I'll give it a look.

 It seems comparable on my machine.

One quick addition, if you look at the the build script for winpty.Net (the nuget package being used for the winpty binaries) it is doing the following:
curl -#Lo winpty-native.zip "https://github.com/rprichard/winpty/releases/download/0.4.2/winpty-0.4.2-msvc2015.zip"
call 7z e -y -o"content\winpty\x86" winpty-native.zip "ia32\bin\winpty.dll" > nul
call 7z e -y -o"content\winpty\x86" winpty-native.zip "ia32\bin\winpty-agent.exe" > nul
call 7z e -y -o"content\winpty\x86" winpty-native.zip "ia32\bin\winpty-debugserver.exe" > nul
call 7z e -y -o"content\winpty\x64" winpty-native.zip "x64\bin\winpty.dll" > nul
call 7z e -y -o"content\winpty\x64" winpty-native.zip "x64\bin\winpty-agent.exe" > nul
call 7z e -y -o"content\winpty\x64" winpty-native.zip "x64\bin\winpty-debugserver.exe" > nul

If you want, you can skip nuget entirely if you just grab the binary from Github (0.4.3 is current as of this comment).

I don't think it's any better really. We still have to download a zip then extract a particular file.

@zacps one additional argument
vtebench -w $(tput cols) -h $(tput lines) -b=100000000 scrolling-in-region --lines-from-bottom=2 > scroll_test.vte

The weird thing to me is that I don't see anything that should obviously cause a difference.

@zacps possibly; what was the motivation for that change?

Nevermind, I realised it's not that because it doesn't affect all benchmarks.
On that note I was not able to reproduce at all.
Given the test generated by @jwilm 's command above I got a difference of less than 100ms (with a total of about 15.7secs).

Using jwilm's command again:

Does that look ok to you? It's going to be hard for me to narrow down the cause when it's this small, if there even is a difference.

@zacps what drove the change of edge to level triggered in the event loop?

@jwilm I think I've addressed all the issues you pointed out.

@zacps thanks! I'll run with a build off this branch until I get an opportunity to bench/final review.
@chrisduerr let's try and avoid landing any major changes until we get this in-tree.

I'm fine with not landing any major changes, however the performance issue is still present.
I'd like to make sure the remaining performance gaps are closed, since there isn't really any good reason why this should impact performance on linux/macos (those users aren't getting any benefit from losing that performance).
In my benchmarks, I've been able to see around 5% performance degradation in the scrolling-in-region benchmark. I wasn't able to see anything in the other benchmarks, however I've just compared single data points instead of a range of results.

@chrisduerr Could you post some information about your system?
My system:
OS: Arch linux
CPU: i5 4690k @4.5GHz
GPU: GTX970 (Proprietary drivers)

Also please try running multiple benchmarks, using the command I ran above with hyperfind makes this a lot easier. Even across 20 runs of ~70sec long tests my branch had a smaller minimum time than master. One run will not give enough information to draw a conclusion.

I'm running a i7 6700k system with the mesa driver (R9 390). Note that I did not run just a single benchmark. The performance degradation is significant and reproducible. And based on what jwilm has said, it doesn't seem like it's something specifically with my system either.
Note that for reproducing this, I would recommend setting a relatively big distance from bottom for the scroll in region benchmark. With my tests that was able to showcase the regression best.

I still can't reproduce, I bumped it up to 5 and 15 lines from bottom and if anything the windows branch performed slightly better (though not a significant difference).

Okay, so I've gone over this and tested things again and again. And I think I'd recommend to go forward with this PR without further investigations of the performance issues.
I've tested running standalone benchmarks, hyperfine, running it through perf and using the benchmarks created in #1653 and I was not able to reliably reproduce any performance degradations.
When running standalone benchmarks there were some interesting fluctuations, like a slowdown of .2s when running the scrolling-in-region benchmark with an offset of 50 after running the asrw benchmark. However I wasn't able to reproduce this consistently and I think that can be mostly blamed on CPU frequency adjustments.
Unless @jwilm is able to reliably reproduce a performance regression, I don't think it's worth to put any more effort into this. Especially because the offset in a scrolling region usually isn't bigger than 1 or 2, so performance regressions with an offset of >10 shouldn't be incredibly critical anyways.

@zacps thanks for all the effort here. I invited you as a collaborator to the project so that you're empowered to improve Windows support and manage issues/PRs pertaining to it.

@zacps Feel free to move the issues over from your tracker and label them with H - windows


Cleanup svg

Awesome loader, exactly what I was looking for moving from my old gulp setup :).
The only thing I'm still looking for is a way to cleanup the SVG with something like https://github.com/svg/svgo. Do you know if this is currently possible? I could work on a PR if not. Let me know.

Okay, my bad. I think I did not look well enough in the output to see that you already do some cleanup. I think I overlooked this because it seems the loader adds some extra stuff as well:
<svg version="1.1" id="Icons" xmlns="http://www.w3.org/2000/svg" data-svgreactloader="[["http://www.w3.org/2000/svg","xlink","http://www.w3.org/1999/xlink"],["http://www.w3.org/2000/svg","space","preserve"]]" x="0px" y="0px" width="21px" height="21px" viewBox="0 0 21 21" enable-background="new 0 0 21 21" xlink="http://www.w3.org/1999/xlink" space="preserve">
	<path d="8 0 1.4.6 1.4 1.4 0 .7-.6 1.4-1.4 1.4zm ... "></path>
</svg>

Is this supposed to happen?
I tried ta add svgo btw, which resulted in the folowing output:
<svg xmlns="http://www.w3.org/2000/svg" width="21" height="21" viewBox="0 0 21 21">
	<path d="8 0 1.4.6 1.4 1.4 0 .7-.6 1.4-1.4 1.4zm ... "></path>
</svg>

I can image even with some sanitising in place, it might be handy to replace that for svgo. Although that means adding another dependency to the project, you do get the benefit of regular patches to svgo (and not having to maintain the sanitise code yourself) plus you could support used defined svgo options (in the webpack config?) so users can benefit from all svgo options.
Here's the patch I used to add svgo, also see https://github.com/tiemevanveen/svg-react-loader/commit/d893659a253c3388f5a3c0166d1689a9f74ea5b4
--- a/index.js
+++ b/index.js
@@ -9,6 +9,8 @@ var assign   = require('lodash/assign');
 var keys     = require('lodash/keys');
 var partial  = require('lodash/partial');
+var SVGO     = require('svgo');

 function readTemplate (callback, filepath) {
     fs.readFile(filepath, 'utf8', function (error, contents) {
         if (error) {
@@ -20,7 +22,17 @@ function readTemplate (callback, filepath) {

 function parseXml (callback, source) {
     var xmlParser = new xml2js.Parser();
-    xmlParser.parseString(source, callback);
+
+    var svgo = new SVGO();
+    svgo.optimize(source, function(result) {
+        if (result.error) {
+            console.log('SVGO failed', result.error);
+            // fall back on source
+            xmlParser.parseString(source, callback);
+        }else{
+            xmlParser.parseString(result.data, callback);
+        }
+    });
 }

@tiemevanveen Apologies for not getting to this earlier.
Those properties are added because, at the time, React didn't handle namespaced attributes, so react-svg-loader placed them in a data-svgreactloader attribute as JSON to be added when the componentDidMount.
So, removing those will break the loader for those using older versions of React.
Also, there is no point in having svg-react-loader doing svgo's work. There is already a loader for svgo you can use that as part of your loader chain, or as a pre-loader.
In addition, the next version of svg-react-loader is in beta, and I want to stop supporting anythig before that.
Again, thanks for trying to contribute and apologies for not replying before you did the work, but these changes are not necessary.

No problem.

Also, there is no point in having svg-react-loader doing svgo's work. There is already a loader for svgo you can use that as part of your loader chain, or as a pre-loader.

Check. That makes sense. How would I do that, could you give me an example?

In addition, the next version of svg-react-loader is in beta, and I want to stop supporting anythig before that.

Oh so the master branch is not the next release.. That's not so handy if it's not mentioned in the docs. User cloning the repo will always checkout master when wanting to contribute.

@tiemevanveen good point -- was just thinking I should add that to the current versions reame...

Would have saved me fixing the tests ;) https://github.com/jhamlet/svg-react-loader/pull/42

Anyway. Keep up the good work, looking forward to v0.4!

done

Check. That makes sense. How would I do that, could you give me an example?

Got it :)
{
        test: /\.svg$/,
        loaders: ['svg-react-loader', 'svgo-loader'],
        exclude: /node_modules/
      }


Error underlined at function definition when calling with arguments of wrong type

Reposting from nuclide flow
https://github.com/facebook/nuclide/issues/356#issuecomment-180456562

Original issue does not have a repro (just a screenshot), but these kinds of errors have been improved recently. Reopen if you have a repro with a bad error message on master.


Fix a little formating issue

It's just a variable not formatted as code.

Overview
Replace this paragraph with a description of your changes and reasoning. Provide links to external references/discussions if appropriate.

Resolves #NN.
Nature of This Change

[ ] 🐛 Fixes a bug
[ ] 🚀 Implements a new feature
[x] 🛁 Rewrites existing documentation
[ ] 💅 Fixes coding style
[ ] 🔥 Removes unused files / code

Related Application Change

Application Pull Request: https://github.com/ExpressionEngine/ExpressionEngine/pulls/NNN

@robsonsobral thanks!


Incorrect translation in Chinese simplified page

EN: The expression sequence[^0] does throw an exception, just as sequence[sequence.Length] does.
CN: 表达式 sequence[^0] 不会引发异常，就像 sequence[sequence.Length] 一样。
The Chinese version page (https://docs.microsoft.com/zh-cn/dotnet/csharp/whats-new/tutorials/ranges-indexes) has a totally opposite meaning, which is obviously incorrect.
Removing character "不" will help.

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

ID: fc6f7384-2a77-a8f2-63dc-c1b545aba2a9
Version Independent ID: eec3706d-1448-9f80-a520-2989b59c1093
Content: Explore ranges of data using indices and ranges
Content Source: docs/csharp/whats-new/tutorials/ranges-indexes.md
Product: dotnet-csharp
Technology: csharp-fundamentals
GitHub Login: @BillWagner
Microsoft Alias: wiwagn

@jmty0083 Thank you for your feedback. I'm following up internally.
@Youssef1313 Thank you for checking. We no longer have loc repos, but I can follow up on localization issues internally, so please just @ mention me going forward.
As I'm following up on this localization issue internally, I'll close this ticket for English repo.


Intelephense: Clear Cache and Reload

Intelephense: Clear Cache and Reload
Command Was Not Found

It has been renamed to 'Index workspace'.
https://github.com/bmewburn/vscode-intelephense/commit/b53210c11e020cbf7a70e53240369eb562f01513#diff-4ac32a78649ca5bdd8e0ba38b7006a1eR45


staged sync

I think it would make sense to move both coreIdCache.js and CoreReplicator.js into the new lib/sync directory I just made today.

awesome, thanks for the comments!
sync seems to be on the root dir, not on lib.But I'll move the CoreIdCache and CoreIdExtension to sync

@tomasciccola This is looking cool! I updated this and the sync branch with latest changes from the authstore branch.
Some tests for CoreIdExtension would be good and then I think we can get this merged.
Since we realized that we only need to send the authstore keys over the extension and the rest of the keys will be shared through blocks in authstore that means the test for CoreIdExtension can be pretty simple, probably just making sure that two peers can exchange their keys (though if you have ideas for other tests I'm not thinking of that would also be good).

yeah, right. I'll update CoreIdExtension and add some tests to it.
So, instead of sharing through it smth like {coreIds:[], namespace: '', identityId:''} we are only sharing the list of coreIds, right? we're not passing neither namespace (since always 'auth') nor identityId (authstore will provide this?)

Right, the extension doesn't need to send the namespace. I'm realizing now that it should send all the coreIds it knows about (not just the identity's cores but all cores), so it could send something like:
[
  {
    coreId: String,
    identityId: String
  },
  ...
]

I think the CoreIdCache is still useful for tracking which cores exist so we can do things like get them all through corestore.get when first loading an app.

I've just been reading through this and thinking about how the extensions and replication will work in practice.
The problem we are solving is that while hypercore will replicate every hypercore that is injected into the replication stream (via core.replicate(stream)), two peers may not have prior knowledge of the public key, which is needed to create the instance of the core in the first place.
The solution here is an "push" approach that sends all keys on first connection. However we need to also handle how we handle additional cores being added to the stream during replicaiton, e.g. Peer A connects to Peer B, sends a list of cores. Then Peer C connects to Peer A, and Peer A gets new cores. Peer A then needs to send any new cores it has received to Peer B. However to know which are "new" then Peer A needs to track what it has already sent to B, unless we just send the complete list. However sending the complete list can result in cycles of traffic, e.g. Peer B now connects to Peer C, so when B receives a new list of cores from A, then it sends a list to C. This is manageable but requires a lot of code to track.
I think a better solution might be to switch to a "pull" approach. Hypercore.createProtocolStream gives us an ondiscoverykey callback that is called for each hypercore which is added to the stream (undocumented unfortunately, like many things in Hypercore). We can register two extensions: "want core" - a message is just a discovery key, and "have core" - a core public key.
For an example, two peers, A & B. A has cores X, Y. B has no cores.

A & B connect.
A adds cores X and Y to the stream
ondiscoverykey() for B will fire with discoveryKey(X) and discoveryKey(Y). This is not enough to start replicating - B needs the public keys for X and Y.
B sends two extension messages: "want core" discoveryKey(X) and "want core" discoveryKey(Y).
A receives these messages, and validates that these cores are auth store keys (since we other public keys are shared in auth store records).
A sends two extension message: "send core" publicKey(X) and "send core" publicKey(Y).
B receives the two public keys, and can now inject them into the stream and replicate them with A

This results in more, smaller messages over the protocol stream, but when two peers are missing core keys. If they each have all the keys then no messages need to be send (ondiscoverykey will fire, and each client will see they already have the key they need).
We can improve message efficiency by just binary encoding of the extension, it can just be a 32-byte buffer (we should validate the buffer length, maybe just ignore and log if we receive not 32-bytes).

I was also looking at how multifeed manages this, and I don't think we need that much complexity. As I understand it, protomux now handles versioning of the protocol (we can check with Mafintosh maybe) and the way our replication works, it doesn't actually need to know about the extension messages if each peer has prior knowledge of the public keys — in that case it is just normal hypercore replication.


DerivedLogger logging level methods don't exist

It seems that these methods don't exist on a DerivedLogger, is this still to do?

Seems that the issue is docs that are not explicit.
  loggerInstance = createLogger({
    levels: config.syslog.levels, // levels is required, not just config.syslog
    level: properties.level,
    format: format.combine(format.colorize(), format.simple()),

    transports: [
      new transports.Console()
    ]
  });

@binarymist if I am correct in understanding the issue I believe this should resolve it https://github.com/winstonjs/winston/pull/1343

Yeah, that looks good. Thanks.


build: use lerna run build instead of lerna exec

Using run build rather than exec allows us to leverage nx build caching

I ran some benchmarks, and it looks like the downside of not running tsc within root context slows the command down by ~3-5 seconds for the initial run.
I think because we don't run this command locally very often (subsequent local runs would be very fast), if we enable nx distributed caching , the benefit we get from CI breezing through the build step could be worth the local build cost?

from CI breezing through the build step

Do we have numbers of how this might happen? Is there a way to test this out without merging to main first?

from CI breezing through the build step

Isn't this a use case for CI caching (think https://github.com/actions/cache or https://docs.gitlab.com/ee/ci/caching/)? If incremental compilation is on and the built files are cached, future builds should be faster because only modified .ts files will be rebuilt.

close in favor of https://github.com/electron/forge/pull/3090/
which uses incremental builds + top level tsc


panel color not displaying correctly

should be light, but it is dark. displayed correctly on other sites.
poco m4 pro 5g
android 13 pixel expirience gsi
kiwi 111.0

This is not a bug . It's the theme color of the website and works the same on other browsers, in this case startpage.com have a theme-color set to #171B25
In future there may be implemented an option in theme to force the theme


What are the "n_frames files", and a small question about "annotation_dir_path "

Hi, two small questions:

What are the "n_frames files"? why do we want to make them?
I didn't understand where the directory with all the annotation, as called in the command: "annotation_dir_path ", suppose to be created because I don't have any directory that includes: "brush_hair_test_split1.txt" or something like that.

I'll be happy for your help.
Thanks.

@Esaada Have you figured out what are the n_frames files? I am confused, too.


Surface converter (and perhaps other functions/etc) at the module level?

Especially with #5 in play, this is now pretty attractive from a performance perspective. Would it be worth exporting the converter at the module level, and perhaps a few other things, to improve usability? And perhaps commit to an API so we can start depending on it externally?

There is an object converter in the module rpy2_arrow.pyarrow_rarrow. It is now mentioned in the doc: https://rpy2.github.io/rpy2-arrow/version/main/html/conversion.html#arrow-lib-table-shared-across-python-and-r
There is likely a ton more to do for usability. What do you have in mind? Moving things up to the __init__.py at the package's root?
Regarding a stable API, the upcoming nanoarrow library would hopefully not change things but the general idea at the moment is to have pyarrow_arrow as the central part.
r6b.py remains a very incomplete exploratory effort at this point. Its use-case would be R6 classes extending the R arrow classes, and provide automatic class representations for Python users as the package rpy2-R6 is attempting to do with its module r6b (IIRC r6a is pretty much the approach you contributed when the project was still on bitbucket). rpy2-R6 is itself not yet fully mature.

Hi again! Yeah, I think at a minimum I'd like to be able to do something like from rpy2_arrow import arrow_converter (the default converter name isn't horrible, but it does mean that everywhere I import it I'd likely be aliasing it.
I think it is fine to still have experimental code present, so long as the bits that are experimental are explicitly called out. Perhaps: "any API directly exposed the top-level module is considered stable and is unlikely to change as we approach 1.0". I'm not sure if there is anything other than the converter that needs to be exposed. At the most I'd imagine some of the helper methods used by the converter in case a manual conversion is desired.
My main goal here is to be able to write a library that depends on rpy2_arrow without the risk of breakage 6 months from now, if/as things mature. If you're not ready to commit to stability, I can always just include a bespoke rpy2 conversion utility in my library as I do now (though likely I'd move away from pre-compiled solutions).

Hi again! Yeah, I think at a minimum I'd like to be able to do something like from rpy2_arrow import arrow_converter (the default converter name isn't horrible, but it does mean that everywhere I import it I'd likely be aliasing it.

The name converter is also used in rpy2.robjects.numpy2ri and rpy2.robjects.pandas2ri. The idea is that it provides a convention (it is always called converter) and the module name disambiguates which converter it is.  For example:
from rpy2.robjects import pandas2ri`

myconverter = pandas2ri.converter + mycustomrules

There are relatively few conversion rule sets around, but what this was trying to make easy
is the creation of rule sets resulting from the stacking different smaller rule sets in
third-party modules with something like:
import functools
import operator

myconverter += functools.reduce(operator.add, m.converter for m in modules)

Having that said a) rpy2.robjects.default_converter does not follow the naming convention,
b) there are not many third-party conversion rule sets at the moment, and c) the conversion
system still have quirks that can sometimes make the stacking of large collections of rule sets surprising.
I am much less happy with the module name pyarrow_rarrow given that one has to write a long-ish import rpy2_arrow.pyarrow_rarrow and alias it because it is long. I see from your comment that you strategy is to import once the converter.
Would the following work for you?
import rpy2_arrow
rpy2_arrow.converter

import rpy2_arrow.core
rpy2_arrow.core.converter

I think it is fine to still have experimental code present, so long as the bits that are experimental are explicitly called out. Perhaps: "any API directly exposed the top-level module is considered stable and is unlikely to change as we approach 1.0". I'm not sure if there is anything other than the converter that needs to be exposed. At the most I'd imagine some of the helper methods used by the converter in case a manual conversion is desired.

Yes. The converter is mostly what user will want. The R of Python type-specific conversion functions can also be occasionally useful. The experimental stuff is in r6b.py (and planned for r6a.py). Each can have their own conversion
rule sets.

My main goal here is to be able to write a library that depends on rpy2_arrow without the risk of breakage 6 months from now, if/as things mature. If you're not ready to commit to stability, I can always just include a bespoke rpy2 conversion utility in my library as I do now (though likely I'd move away from pre-compiled solutions).

The previous release was 8 months ago, so passing the 6-month mark is definitely do-able. :-)
Otherwise, the performance improvement you and @paleolimbot worked out did not change the API so it has been stable for longer than that.
At the moment the only change could be to add to rpy2_arrow/__init__.py something like:
from rpy2_arrow.pyarrow_rarrow import converter

Or may be rename pyarrow_rarrow to core.
r6a and r6b can keep living as experimental in their own modules.  At the moment the r6a and r6b approaches are mutually exclusive and build atop the conversion rules in pyarrow_rarrow.
I am also happy to add you as a PR reviewer in the project's settings.

I forgot the R arrow, narrow, nanoarrow part. Currently the conversion is using R arrow. The conversion function would map to different classes if narrow/nanoarrow is used in the future. A more stable option would that rpy2-arrow
has a module r_arrow for the current conversion rules and a module nanorrow added later. This would mean that pyarrow_rarrow.py is renamed to r_arrow, but no default converter is added to __init__.py for the time being.

Would the following work for you?
import rpy2_arrow
rpy2_arrow.converter

import rpy2_arrow.core
rpy2_arrow.core.converter

Yes. Of these I prefer the first :).

At the moment the only change could be to add to rpy2_arrow/__init__.py something like:
from rpy2_arrow.pyarrow_rarrow import converter

Or may be rename pyarrow_rarrow to core.

I like the core name even if we don't expect people to directly import it due to the above. That said, it would be a breaking change, and I'm not sure it's worth it if we aren't expecting it to be part of the public API.

I am also happy to add you as a PR reviewer in the project's settings.

I'm happy to help review things (but don't gate things on me if I'm sow to respond; I trust your judgement).

I forgot the R arrow, narrow, nanoarrow part. Currently the conversion is using R arrow. The conversion function would map to different classes if narrow/nanoarrow is used in the future. A more stable option would that rpy2-arrow
has a module r_arrow for the current conversion rules and a module nanorrow added later. This would mean that pyarrow_rarrow.py is renamed to r_arrow, but no default converter is added to __init__.py for the time being.

I thought that nanoarrow was more just helpers for casting datatypes, and the underlying types themselves were the same. If that is true, then I suspect you could just use their conversion helpers instead of your own here (or not if you are happy with directly using the c interfaces). If I'm missing something, maybe we'd have to add a new module, but at this stage I doubt it would make sense (under the hood we'd still just be exporting to the c interface and back again).

At the moment the only change could be to add to rpy2_arrow/__init__.py something like:
from rpy2_arrow.pyarrow_rarrow import converter

Or may be rename pyarrow_rarrow to core.

I like the core name even if we don't expect people to directly import it due to the above. That said, it would be a breaking change, and I'm not sure it's worth it if we aren't expecting it to be part of the public API.

The name pyarrow_rarrow is unnecessarily long. It makes sense to change it, I think.
With respect to API-breaking changes, rpy2-arrow is still quite early in its development. Also, having a deprecated module pointing to the new module can be done.

I am also happy to add you as a PR reviewer in the project's settings.
I'm happy to help review things (but don't gate things on me if I'm sow to respond; I trust your judgement).

I'd be more concerned about lag on my end ;-). The idea here is that I am not confident to guarantee that API-breaking changes will never happen. Having reviewers introduces a feedback loop during which more perspectives are considered. For example, is the change worth the trouble, what is the likely impact on existing code relying on it, and what is the easiest & safest way to migrate to it. If you are using this in prod and/or at scale you can be an important representative for this.
We can also figure out what is the right timeout parameters beyond which a change is probably fine to carry out.
Having that said, I think that this is getting stable. As mentioned the latest update to improve performance did not break the API. I'd expect possible future API changes to be forced by changes upstream:

rpy2 and its conversion system. This might break custom converters.
pyarrow, although API-breaking changes would only appear if the API to Arrow changes classes converted from and to.
R arrow, with a similar comment to the one above about pyarrow.
Having adapter code (and DeprecationWarning raised) during a grace period should be feasible in almost all cases in order to guarantee stability over periods of time and facilitate the migration to a new API.

I forgot the R arrow, narrow, nanoarrow part. Currently the conversion is using R arrow. The conversion function would map to different classes if narrow/nanoarrow is used in the future. A more stable option would that rpy2-arrow
has a module r_arrow for the current conversion rules and a module nanorrow added later. This would mean that pyarrow_rarrow.py is renamed to r_arrow, but no default converter is added to __init__.py for the time being.

I thought that nanoarrow was more just helpers for casting datatypes, and the underlying types themselves were the same. If that is true, then I suspect you could just use their conversion helpers instead of your own here (or not if you are happy with directly using the c interfaces). If I'm missing something, maybe we'd have to add a new module, but at this stage I doubt it would make sense (under the hood we'd still just be exporting to the c interface and back again).

I am not familiar enough with narrow and nanoarrow. narrow is an R package that was brought up to my attention as an easier R package to install than arrow. Less system dependencies to work out, IIRC. The mental model I have so far is that narrow/nanoarrow are closer to the C API to pass around pointers, as well as cast or transform Arrow data structures to other Arrow data structures. arrow has implicitly some of that, but has more system dependencies (narrow / nanoarrow include headers & a C library that is a subset of the full Arrow libraries) as well as R6 class wrappers to interact with Arrow data structures and functions from R (@paleolimbot  @eddelbuettel - can you confirm that I got this right?).

Yes. You can interface "Arrow" (the data model) without arrow (the package, and its sometimes rather involved compilation steps -- which have gotten better but it is still a sizeable package with, on my machine, a 40+mb shared library) because there is a 'zero-dependency' interface at the C level with just two void pointers to two C level struct objects.  Look e.g. at duckdb for use of that.  @paleolimbot had a series of package attempts around, ending at narrow which had its last commits in February.  I have some small extensions on that.  I found it very helpful for lighterweight tests and CI use.
It now seems that we may get an officially blessed and supported package named nanoarrow which is reportedly coming "Real Soon Now (TM)" and I look forward to working with it.

Thanks for specifying that there is no overlap between pyarrow and narrow/nanoarrow.
rpy2-arrow's purpose is to make the sharing of arrow data structures between Python and R easier. For example, allowing the creation of an instance for an Arrow class in Python, and then pass it to R to be used in R code that can handle Arrow data structures.
It can consider pyarrow and (R's) arrow as a pair with rpy2 conversion rules, and nanoarrow (Python package of that name) and nanoarrow (R package) another pair with other conversion rules.

With that context, then; it seems like from rpy2_arrow import converter and from rpy2_arrow import nanoarrow_converter would make sense? Or do you always want the .converter naming? In any case, I think making it accessible from the toplevel module makes sense, and I'm happy with any naming you come up with :).

With that context, then; it seems like from rpy2_arrow import converter and from rpy2_arrow import nanoarrow_converter would make sense? Or do you always want the .converter naming? In any case, I think making it accessible from the toplevel module makes sense, and I'm happy with any naming you come up with :).

It would be easier to have the converter requiring pyarrow/R's arrow and the possible future one requiring nanoarrow (Python and R packages) in different modules. The reason for that the R (and Python) packages would be loaded in R when the module is imported, and I don't think this should require both pyarrow/arrow and nanoarrow to be installed and loaded.
The name converter within a module, this could be
import rpy2_arrow.arrow
# converter:
rpy2_arrow.arrow.converter

# or

from rpy2_arrow import arrow
# converter:
arrow.converter

or
import rpy2_arrow.nanoarrow
# converter:
rpy2_arrow.nanoarrow.converter

# or

from rpy2_arrow import nanoarrow
# converter:
nanoarrow.converter

With the above, the name could conflict with the future Python package nanoarrow
if a user want to have import nanoarrow is the same module. However, the same argument
could be made about any subpackage or module named core.

Roger that! I'm happy with those choices.
The nanoarrow option could also be rpy2_arrow.narrow if you wanted to avoid the name collision and/or type less.
Thanks @lgautier !

Feel free to close this issue out now. I'm happy with the state of main. Thank you!

You got it. Looking forward to more issue reports as you battle-test this. :-)


Add F# types to Language Specific Names table

Summary
List F# type names in the Avoiding Language-Specific Names table.

 Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.:x: simontaite sign nowYou have signed the CLA already but the status is still pending? Let us recheck it.

Thanks for your contribution @simontaite. Unfortunately, I'll have to close this PR since this portion of the docs is copyrighted and it's from the book "Framework Design Guidelines: Conventions, Idioms, and Patterns for Reusable .NET Libraries, 2nd Edition". We're trying to understand if we can make updates to it.


Migrate benchmark to criterion.

I'd be interested in working on this, but am currently having trouble finding out where the benchmarks are located currently >_>

It is very sparse, but they tend to be in their own module & crates.
) rg "\#\[bench\]"
src/store/mod.rs
135:    #[bench]
146:    #[bench]

src/query/union.rs
426:    #[bench]
443:    #[bench]

src/fastfield/mod.rs
446:    #[bench]
459:    #[bench]
472:    #[bench]
506:    #[bench]

src/common/bitset.rs
364:    #[bench]
377:    #[bench]
385:    #[bench]
391:    #[bench]

src/postings/mod.rs
638:    #[bench]
652:    #[bench]
717:    #[bench]
722:    #[bench]
727:    #[bench]
732:    #[bench]
737:    #[bench]

src/collector/facet_collector.rs
615:    #[bench]

src/query/bitset/mod.rs
230:    #[bench]
242:    #[bench]
253:    #[bench]

src/postings/compression/mod.rs
291:    #[bench]
300:    #[bench]
326:    #[bench]
335:    #[bench]

src/query/phrase_query/phrase_scorer.rs
294:    #[bench]
303:    #[bench]

src/postings/stacker/expull.rs
318:    #[bench]
334:    #[bench]

It is trickier than it might initially looks.
You can also have a look at other tantivy-search crates.

Hello,
I've recently used criterion in a personal project and I'd be glad to give a shot at this migration if possible. However, I don't know if @cronokirby is already working on this.
In terms of organization, is the idea to keep the benchmarks where they are (sparse through the relevant rust files) or to gather them inside a benches root-folder (an example with only one benchmark moved)?
The answer might depend on how much the benchmarks depend on internal structures.


Adding abi_stable support for tremor-value, v3

Pull request
Description
This is an intermediate PR for #1287 so that it's more manageable.
This is the third attempt for tremor-value, see #1303 for the first one.

Second to third version was due to conflict issues, so I cherry-picked the PR's commits manually and recreated it
First to second version was explained in #1303:

After talking with @darach, we both agreed that this might not be the best way to do it. Instead of having a single Value that's now abi_stable compatible, I suggest having two of them: one for the PDK, and another for regular usage. We can then convert to and from the regular type into the PDK type in order to communicate with the plugins.

It's much simpler: abi_stable was propagating throughout the entire Tremor codebase and it was quickly getting out of hands. I was having trouble trying to make tremor-script compile, because it also uses Value for reasons outside of the PDK. I basically had to rewrite lots of stuff just for the PDK.
Less importantly: performance. At first it seems like it might be more costly, because having two types means there's a conversion from the regular value to the PDK value in order to pass it to the plugin, and then the inverse to use it in the plugin again. But it turns out that by using a single Value we were missing out on lots of optimizations, such as known_key, raw_entry, the memory efficiency of beef::Cow, or the performance of halfbrown::HashMap. And we were performing lots of conversions as well anyway, so it's not like this was better.

So yeah, I'm not fully sure having two value types is going to be better, but I'm certain it's worth trying. @Licenser was absolutely right that I shouldn't worry much about this PR because I was probably going to end up rewriting most of it anyway haha. At least having value-trait be abi_stable-compatible is still useful because I'll use it for the PDK value.
I'll try to rewrite this from almost scratch and let you know when I'm more or less done :)

In this PR, all I do is make tremor-value compatible with abi_stable. The main crate will still compile because it's a backward-compatible change. I'll soon make another PR with the changes to tremor-runtime, which are much more complicated than this simple PR.
Once this is approved and more or less ready, I'll merge https://github.com/marioortizmanero/tremor-runtime/pull/3 over it with the full PDK changes.
Related

RFC
Related Issues: #791 #1077
Related docs: https://nullderef.com/series/rust-plugins/

Checklist

[x] The RFC, if required, has been submitted and approved
[ ] Any user-facing impact of the changes is reflected in docs.tremor.rs
[ ] The code is tested
[ ] Use of unsafe code is reasoned about in a comment
[ ] Update CHANGELOG.md appropriately, recording any changes, bug fixes, or other observable changes in behavior
[ ] The performance impact of the change is measured (see below)

Performance

Fixed that and updated the pdk.rs file in tremor-value from the changes for tremor-runtime

Could you add a small both to satisfy the gods of coverage and also to rule out any surprises?

Another rebase with crazy changes, starting from scratch at #1381


WFLY-4574 Update JSF based on Mojarra 2.2.11

https://issues.jboss.org/browse/WFLY-4574

Windows Build 1515 is now running using a merge of f087e60a9823ab05b8840252250f2cb6ca0e09a6

Linux Build 6436 is now running using a merge of f087e60a9823ab05b8840252250f2cb6ca0e09a6

Linux with security manager Build 1343 is now running using a merge of f087e60a9823ab05b8840252250f2cb6ca0e09a6

Linux with security manager Build 1343 outcome was SUCCESS using a merge of f087e60a9823ab05b8840252250f2cb6ca0e09a6
Summary: Tests passed: 821, ignored: 318 Build time: 0:06:09

Linux Build 6436 outcome was SUCCESS using a merge of f087e60a9823ab05b8840252250f2cb6ca0e09a6
Summary: Tests passed: 2863, ignored: 373 Build time: 0:48:48

Windows Build 1515 outcome was SUCCESS using a merge of f087e60a9823ab05b8840252250f2cb6ca0e09a6
Summary: Tests passed: 2863, ignored: 373 Build time: 0:52:27


Some Advices

At first I use es-rally tool to run benchmark on es, actually, it really not working so good .
After I found this project, I like the theory you put here: https://logz.io/blog/benchmarking-elasticsearch-magnetic-ebs/?utm_content=buffer804f1&utm_medium=social&utm_source=twitter.com&utm_campaign=buffer
Such as it use docker, and I can launch instance as many as I want.
But I have few questions:

The metrics in Graphite is not easy to understand, can you provide a table with description.

Actually I use Grafana to visualize the status of es, can you provide a Grafana template?

The parameter like 'SERVICE_HOST' is not very clearly, I really don't know what it meas.

Hey @jackpgao and thanks for the feedback!

I see. Tried to name the metrics as descriptive as I can. Anyone, in particular, is not clear?
I'll see if i can get something out. but for now, its just 5/6 metrics so should be hard to build a dashboard for them
Is taken from this https://github.com/logzio/jmx2graphite . Basically, your server name

Hi, @roiravhon , thanks for you reply.
Actually I have few Questions:

the metric 'averageSearchTimeMs', it seems a current value, which means the avg search response time .

the metric 'totalSuccessfulSearchsTimeMs', it seems a total value of search time, I should use totalSuccessfulSearchsTimeMs/(numberOfSuccessfuleSearchs+ numberOfFailedSearchs) as the avg query time, is it right?

The parameter 'SERVICE_HOST', is it the same with the parameter 'elasticsearchAddress' in  conf file?

you are right about 1 & 2.
Actually, the average is calualated exactly like that - see here https://github.com/logzio/elasticsearch-benchmark-tool/blob/master/src/main/java/io/logz/benchmarks/elasticsearch/metrics/SearchMbean.java#L52
And 3 - this is totally up to you. This will be the prefix at graphite, for your convenience

So can this be closed @jackpgao ?


Soundcloud unable to download songs

Checklist

[x] I'm reporting a broken site support
[x] I've verified that I'm running youtube-dl version 2019.06.27
[x] I've checked that all provided URLs are alive and playable in a browser
[x] I've checked that all URLs and arguments with special characters are properly quoted or escaped
[x] I've searched the bugtracker for similar issues including closed ones

Verbose log

[debug] System config: []
[debug] User config: []
[debug] Custom config: []
[debug] Command-line args: ['https://soundcloud.com/marshmellomusic/marshmello-x-sob-x-rbe-roll-the-dice?in=soundcloud-the-peak/sets/edm-crossovers', '-v']
[debug] Encodings: locale cp1252, fs mbcs, out cp437, pref cp1252
[debug] youtube-dl version 2019.06.27
[debug] Python version 3.4.4 (CPython) - Windows-10-10.0.17134
[debug] exe versions: none
[debug] Proxy map: {}
[soundcloud] marshmellomusic/marshmello-x-sob-x-rbe-roll-the-dice: Downloading webpage
WARNING: Unable to download webpage: HTTP Error 500: Internal Server Error
Traceback (most recent call last):
  File "__main__.py", line 19, in <module>
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\ytdl-org\tmpjzs7t34q\build\youtube_dl\__init__.py", line 474, in main
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\ytdl-org\tmpjzs7t34q\build\youtube_dl\__init__.py", line 464, in _real_main
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\ytdl-org\tmpjzs7t34q\build\youtube_dl\YoutubeDL.py", line 2008, in download
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\ytdl-org\tmpjzs7t34q\build\youtube_dl\YoutubeDL.py", line 796, in extract_info
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\ytdl-org\tmpjzs7t34q\build\youtube_dl\extractor\common.py", line 530, in extract
  File "C:\Users\dst\AppData\Roaming\Build archive\youtube-dl\ytdl-org\tmpjzs7t34q\build\youtube_dl\extractor\soundcloud.py", line 446, in _real_extract
UnboundLocalError: local variable 'info_json_url' referenced before assignment

Description

I receive this error on any Soundcloud track I attempt to download, whether or not it is in a playlist. This started recently as I was able to download from Soundcloud earlier without issue.

@dstftw Can you point me to the similar issue I must have missed? I tried searching for Soundcloud issues but they all were very outdated at this point.


Split tax - Tax amount is negative when manually entering tax amount when splitting expense

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

Version Number: 9.0-3.2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4678911
Issue reported by:  Applause - Internal Team
Action Performed:
Precondition:

Taxes feature is enabled.
Workspace has at least one non-zero tax rate

Go to staging.new.expensify.com
Go to workspace chat
Go to + > Split expense > Manual
Enter amount > Next
Click Show more > Tax rate
Select a non-zero tax rate
Click Tax amount
Manually enter the tax amount
Split the expense
Click on the split preview

Expected Result:
The tax amount will not be negative
Actual Result:
The tax amount is negative when manually entering tax amount when splitting expense
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

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

Screenshots/Videos
Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/4551dcf0-6163-4320-93e0-874312a2ba63
View all open jobs on GitHub

@CortneyOfstad  FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

We think that this bug might be related to #wave-collect - Release 1

Proposal
Please re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
BE returns negative tax amount when editing tax amount manually. And  we convert the negative tax amount to formatted tax amount here in MoneyRequestConfimationListFooter without first converting the tax amount into positive value as we do in MoneyRequestView.
https://github.com/Expensify/App/blob/937c4186409725b57fdaae6ae8e1d8c85a9d52a0/src/components/MoneyRequestConfirmationListFooter.tsx#L254
And display the formatted value here
https://github.com/Expensify/App/blob/937c4186409725b57fdaae6ae8e1d8c85a9d52a0/src/components/MoneyRequestConfirmationListFooter.tsx#L484-L487
What changes do you think we should make in order to solve the problem?
As we do in MoneyRequestView, we should first convert the taxAmount to positive following the same method used in Money RequestVeiw
const taxAmount = TransactionUtils.getTaxAmount(transaction, false);
getTaxAmount() returns the absolute value of transaction.taxAmount
Then we format it
const formattedTaxAmount = CurrencyUtils.convertToDisplayString(taxAmount, iouCurrencyCode);
here
https://github.com/Expensify/App/blob/937c4186409725b57fdaae6ae8e1d8c85a9d52a0/src/components/MoneyRequestConfirmationListFooter.tsx#L254C11-L254C29
What alternative solutions did you explore? (Optional)

Proposal
Please re-state the problem that we are trying to solve in this issue.
The tax amount is negative when manually entering tax amount when splitting expense
What is the root cause of that problem?
If we create a split bill with entering the tax amount manually, BE returns the tax amount as negative amount for expense report.
And in here, we doesn't convert the tax mount to positive value before getting the formatted string.
https://github.com/Expensify/App/blob/5cebd3ebb70e95a66ecdd60b3057027b619da39a/src/components/MoneyRequestConfirmationListFooter.tsx#L254
What changes do you think we should make in order to solve the problem?

If we create a split bill with entering the tax amount manually, BE returns the tax amount as negative amount for expense report.

With this case BE returns the tax amount as negative amount for expense report. But if we don't enter the tax amount manually, BE returns it as positive amount. So here we should use getTaxAmountAbsValue to make sure that the displayed tax amount is always positive value
const formattedTaxAmount = CurrencyUtils.convertToDisplayString(ModifiedExpenseMessage.getTaxAmountAbsValue(transaction?.taxAmount ?? 0), iouCurrencyCode);

https://github.com/Expensify/App/blob/5cebd3ebb70e95a66ecdd60b3057027b619da39a/src/components/MoneyRequestConfirmationListFooter.tsx#L254
What alternative solutions did you explore? (Optional)
We can fix from BE side to always return the tax amount of split bill transaction in expense report as negative amount
And then here, we can use getTaxAmount function that had already covered the negative amount case for expense chat.
const formattedTaxAmount = CurrencyUtils.convertToDisplayString(TransactionUtils.getTaxAmount(transaction, isPolicyExpenseChat), iouCurrencyCode);

https://github.com/Expensify/App/blob/5cebd3ebb70e95a66ecdd60b3057027b619da39a/src/components/MoneyRequestConfirmationListFooter.tsx#L254

I was able to recreate so going to get eyes on this!

@aimane-chnaif there are two proposals here and here for review when you have a chance — thank you!

@aimane-chnaif I can't find.

 I hope you get better now.

PR will be ready for review in few hours.

@aimane-chnaif — please complete the check ASAP so there is no further delay in payment. Thanks!
@etCoderDysto — I sent you an offer in Upwork, please let me know once you accept and I will get that paid ASAP. Thanks!

@CortneyOfstad  I have accepted the offer.
Thank you!

Payment Summary
@etCoderDysto — paid $250 via Upwork
@aimane-chnaif — to be paid $250 via NewDot
Thanks all!


Add html string injections, make queries more advanced

The first two injections are for HTML. I updated the queries to try and eliminate false positives, but there are still some that exist. WITH is still pretty barebones, and EXPLAIN is the same...do you know how to define variables in scheme? I tried (define (select "...")) but that didn't work. If we could define vars, that would make this way easier...and explain could be made better.
I also removed REPLACE since it's a function and not a keyword. I can put it back if you want, but I don't think there's a way for it to be highlighted correctly with how it was set up.
here are some examples

Interesting!
Does it works if queries contains new lines ?

Interesting! Does it works if queries contains new lines ?

It does, I had tested that but guess I erased that in the screenshot.

As far as I know, this is not possible to create variables in scheme 😅

Well, that's too bad. Would make this a lot easier instead of having to keep up with all the different types of strings. Oh well!

Thanks !!


Replace npmignore by exclusion filter in package.json

Ref https://github.com/stylelint/stylelint/pull/333

Done.


transfer process stopped working after update

Bug Report
Describe the Bug
The commit 988215e7094778495938ff79e6bbfc038b9e8981 broke our usage of the API endpoints /api/v1/data/transferprocess and /api/v1/data/catalog.
Expected Behavior
Afterwards, the endpoint /api/v1/data/catalog returns 200/OK and a benevolent JSON in its body.
Observed Behavior
The endpoint returns 500/Internal Server Error and a HTML body stating Internal Server Error
Steps to Reproduce

Start negotiation in order to get agreement and register asset (works)
Transfer process via /api/v1/data/transferprocess (which returns 200/OK)
Look for asset in catalog via /api/v1/data/catalog.

Context Information

Used version: 988215e7094778495938ff79e6bbfc038b9e8981 (also happens for as-of-now current 3ff940b720f44826df28e893fb31344eb6faacef)
OS: Windows 10

Detailed Description
The log gives stack trace

org.eclipse.dataspaceconnector.spi.EdcException: Received unexpected response type.
at org.eclipse.dataspaceconnector.ids.api.multipart.dispatcher.sender.IdsMultipartSender.checkResponseType(IdsMultipartSender.java:332)
at org.eclipse.dataspaceconnector.ids.api.multipart.dispatcher.sender.IdsMultipartSender.lambda$send$0(IdsMultipartSender.java:210)
at org.eclipse.dataspaceconnector.ids.core.message.FutureCallback.onResponse(FutureCallback.java:45)

This error message is thrown already after /api/v1/data/transferprocess; the 500 of /api/v1/data/catalog is likely only a consequence.

I cannot reproduce the exception. I used sample 4.0 to test the negotiation and transfer and thought that it may be because the IDS object mapper is used for payload de/serialization in MultipartEndpointDataReferenceRequestSender but not in EndpointDataReferenceHandler. However, this seems not to be the issue.
As with commit c38d744 the response type check was introduced, the connector now does not handle RejectionMessages as "okay", as it did before. In this course, we also recognized some failing tests etc. that checked for a response, but not for a successful one. @bscholtes1A Any other idea why the transfer could fail? I guess the "unexpected response type" is a RejectionMessage.
I propose to open up a PR that resets the ObjectMapper in the MultipartEndpointDataReferenceRequestSender and detail the "Received unexpected response type." log message. But I am not sure that this will help.

@kedorlaomer Can you maybe provide some more information about what DPF and transfer protocol you are using?

It is a RejectionMessage. I don't know yet where it originates.

I can send you a recording of our use of these endpoints:
rejected-flow.txt
negotiation-flow.txt
The communication in the file seems OK, although I'm a bit concerned about the POST request to /api/v1/data/transferprocess: Why does dataDestination have an empty "type"? Do you think this is to be blamed?
In any case, we can see that it is a RejectionMessage by the connector.

I think, I can pinpoint the problem quite clearly: The consumer POSTs to /api/v1/ids/data something like
    {
      "ids:recipientConnector": [
        {
          "@id": "http://localhost:8282/api/v1/ids/data"
        }
      ],
      "ids:modelVersion": "4.1.3",
      "ids:issued": {
        "@value": "2022-08-16T11:16:13.104+02:00",
        "@type": "http://www.w3.org/2001/XMLSchema#dateTimeStamp"
      }, ...

and receives a HTTP/200 with a multipart response encapsulating a DescriptionResponseMessage and a ConnectorEndpoint statement:
    {
      "ids:hasDefaultEndpoint": {
        "@type": "ids:ConnectorEndpoint",
        "@id": "https://w3id.org/idsa/autogen/connectorEndpoint/41b9999c-0257-4739-9029-1a3aafd9e194",
        "ids:accessURL": {
          "@id": "https://example.com"
        }
      },
      "ids:securityProfile": {
        "@id": "https://w3id.org/idsa/code/BASE_SECURITY_PROFILE" ...

This object has a field  "ids:resourceCatalog" which contains a "ids:offeredResource" list. That list does not have a "@type" field. The JSON parser does not find the "@type" and therefore does not know which object type to generate. This yields a stack trace such as
    org.eclipse.dataspaceconnector.spi.EdcException: Could not deserialize connector self-description: Could not resolve subtype of [simple type, class de.fraunhofer.iais.eis.Resource]: missing type id property '@type' (for POJO property 'ids:offeredResource')
     at [Source: (byte[])"{"ids:hasDefaultEndpoint":{"@type":"ids:ConnectorEndpoint","@id":"https://w3id.org/idsa/autogen/connectorEndpoint/41b9999c-0257-4739-9029-1a3aafd9e194","ids:accessURL":{"@id":"https://example.com"}},"ids:securityProfile":{"@id":"https://w3id.org/idsa/code/BASE_SECURITY_PROFILE"},"ids:resourceCatalog":[{"ids:offeredResource":{"objectList":[{"ids:contractOffer":[{"ids:permission":[{"ids:target":{"@id":"urn:artifact:asset-for-asset-management"},"ids:action":[{"@id":"https://w3id.org/idsa/code/USE","[truncated 2388 bytes]; line: 1, column: 2869] (through reference chain: de.fraunhofer.iais.eis.BaseConnectorImpl["ids:resourceCatalog"]->java.util.ArrayList[0]->de.fraunhofer.iais.eis.ResourceCatalogImpl["ids:offeredResource"]->java.lang.Object[0])
            at org.eclipse.dataspaceconnector.ids.api.multipart.dispatcher.sender.MultipartCatalogDescriptionRequestSender.getBaseConnector(MultipartCatalogDescriptionRequestSender.java:146)
            at org.eclipse.dataspaceconnector.ids.api.multipart.dispatcher.sender.MultipartCatalogDescriptionRequestSender.getResponseContent(MultipartCatalogDescriptionRequestSender.java:114)
            at org.eclipse.dataspaceconnector.ids.api.multipart.dispatcher.sender.IdsMultipartSender.lambda$send$0(IdsMultipartSender.java:205)
            at org.eclipse.dataspaceconnector.ids.core.message.FutureCallback.onResponse(FutureCallback.java:45)
            at okhttp3.internal.connection.RealCall$AsyncCall.run(RealCall.kt:519)
            at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
            at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
            at java.base/java.lang.Thread.run(Thread.java:833)

This can be reproduced in an even simpler way:

Start two connectors
Call the catalog endpoint via
curl -v 'http://localhost:9192/api/v1/data/catalog/?providerUrl=http://localhost:8282/api/v1/ids/data'
where localhost:9192 is the IDS endpoint and localhost:8282 is the data endpoint

I tried PR 1790 and it did not solve the problem.
I found the following: When JsonLdSerializer tries to serialise a de.fraunhofer.iais.eis.BaseConnector, it correctly adds a @type annotation. But when recursing down to serialise the de.fraunhofer.iais.eis.ResourceCatalog, it uses a BeanSerializer instead of the correct JsonLdSerializer (which knows how to add the @type/@context annotation) which does not add @type annotations to its children (e. g. the ids:offeredResource where the error occurs). I did not dig further into how the serialiser is being passed down the chain, but I'm convinced the error is in that area.

After a lot of debugging, the reason was found to be incompatible versions of Fraunhofer's infomodel. This led to a ResourceCatalog which contained values that the new serialiser was not equipped to serialise. Therefore, the more generic BeanSerializer was used instead, leading to the observed RejectionMessage. We solved this by adapting our own version of the infomodel. Thank you, @juliapampus for your help!


✨ [Go SDK] Introduce interfaces for interoperability and encapsulation

What are you trying to do?
I'm trying to build a set of libraries on top of the Go SDK to provide higher level interfaces to Dagger making it easier for people to work with it.
Some of those libraries/features may require to encapsulate dagger.Client or dagger.Container in custom interfaces. For example: I have a custom Client that detects if Dagger is running on GitHub Actions and forwards all GitHub related environment variables automatically to all Containers.
In order to do this, I've created a Client interface that I use internally (exposing all methods of dagger.Client). The problem with this approach is that it's not interoperable between different libraries that depend on dagger.Client.
Why is this important to you?
In order to be able to encapsulate certain parts of the SDK and keep interoperability between different libraries.
How are you currently working around this?
I have a custom interface at the moment and I use it in all of my libraries instead of depending on dagger.Client directly, but there are no guarantees that every potential library out there will do the same.

Hi, have you seen how this could work for you?

#3885

I think it's a nice syntactic sugar, but it still wouldn't allow encapsulating any of those resources.
The use case is also a bit different: encapsulation would support passing the client and containers across API layers (eg. using multiple, different libraries)

Here is the repo, but that part of the code is not pushed yet: https://github.com/sagikazarmark/goci

A use case you shared, detecting if Dagger is running via GitHub Actions, should be supported as a first-class state by Dagger. Taking that info and using it throughout the pipeline is critical.

Agreed.

@d3rp3tt3 here: https://github.com/sagikazarmark/goci/pull/2

Thank you, @sagikazarmark! The code is really clean and easy to read for me.
I see you're adding several pieces of GitHub metadata as environment variables with the container. How do you use that info?

I would also appreciate if you could provide the mentioned interfaces for the client and the other types. Usecase is similar to what @sagikazarmark described. For compliance reasons I'm building a little framework around dagger with the intend to automatically capture build logs in a certain format, checksums of the images being used, etc. Those interfaces would enable me to mock interactions with the dagger framework and eventually speed up testing.

@v0lkc Thanks for your time today! As mentioned on the call, please share a summary of your use case here. This will help us for the next steps of the PR review.
For feature requests, we want to support them in Dagger proper vs one SDK as much as possible, so we will take that into consideration when we are evaluating the PR and your use case.

@mircubed Do we have any update on this issue?

Considering that we now have https://docs.dagger.io/zenith/, and users are able to create & share their own modules, we are thinking that is less important and perhaps even unnecessary.
If you agree with the above @sagikazarmark, would you be OK to close this?
If we don't close this, it will not be before sometime in 2024 that we can follow-up on this considering everything else that is in flight.
Thank you! In-sync with @helderco. cc @vito

Considering that we now have https://docs.dagger.io/zenith/, and users are able to create & share their own modules, we are thinking that is less important and perhaps even unnecessary.

Agreed. Depending on whether the plain Go SDK remains a first-class citizen, it's either less important or unnecessary.

Just a side note that Zenith modules will support interfaces in the very near future: https://github.com/dagger/dagger/issues/6213
That change specifically will most likely just start as an optional field, totally backwards compatible, but the option of going "full-blown everything is an interface" remains open in my mind, especially if it can be done fully or at least mostly backwards compatible and provides enough benefits.
At that time we'll need to make a call on whether it's Zenith Go SDK only or applies to the "legacy" one too.

Hey @sagikazarmark would you consider this resolved with modules now?

I guess so. Zenith introduced different challenges, but I guess this isn't the right issue to track those.


RC Spot-Check Test: energy-skate-park 1.2.0-rc.2

RC Spot-Check Test
Mentions: @kathy-phet and @KatieWoe
Simulation links

github repository for issues
phet top level directory
sim: all_phet.html

Test Matricies
General Test:

[x] Tester = NS, Platform = mac+ safari; iPhone , Time = 1hr
[x] Tester = KW, Platform = Win 11 Chrome, Time = .5

Features included
Focus and Special Instructions

Issues to Verify
These issues should have the "status:ready-for-review" label. Unless an issue says to close after verifying, assign the
issue back to the developer.
These ones say "status:fixed-awaiting-for-deploy" to not confuse them with the "ready-for-review" that wont go in the milestone. Please close after verifying unless the issues still persist.

[x] https://github.com/phetsims/energy-skate-park/issues/372
[x] https://github.com/phetsims/energy-skate-park/issues/369
[x] https://github.com/phetsims/energy-skate-park/issues/368
[x] https://github.com/phetsims/energy-skate-park/issues/367
[x] https://github.com/phetsims/energy-skate-park/issues/366
[x] https://github.com/phetsims/energy-skate-park/issues/362
[x] https://github.com/phetsims/energy-skate-park/issues/349

For QA...

General features

What to Test

Click every single button.
Test all possible forms of input.

Test all mouse/trackpad inputs.
Test all touchscreen inputs.

If there is sound, make sure it works.
Make sure you can't lose anything.
Play with the sim normally.
Try to break the sim.
Test some query parameters. (See QA Book for a list of query parameters.)
When making an issue, check to see if it was in a previously published version.
Try to include version numbers for browsers.
If there is a console available, check for errors and include them in the Problem Description.
As an RC begins and ends, check the sim repo. If there is a maintenance issue, check it and notify developers if there is a problem.
As the RC ends, notify the developer of any new QA credits that need to be added.

PhET-iO features

What to Test

Make sure that public files do not have password protection. Use a private browser for this.
Make sure that private files do have password protection. Use a private browser for this.
Make sure standalone sim is working properly.
Make sure the wrapper index is working properly.
Make sure each wrapper is working properly.
Launch the simulation in Studio with ?stringTest=xss and make sure the sim doesn't navigate to youtube
For newer PhET-iO wrapper indices, save the "basic example of a functional wrapper" as a .html file and open it. Make
sure the simulation loads without crashing or throwing errors.
Load the login wrapper just to make sure it works. Do so by adding this link from the sim deployed root:/wrappers/login/?wrapper=record&validationRule=validateDigits&&numberOfDigits=5&promptText=ENTER_A_5_DIGIT_NUMBER

Further instructions in QA Book
Conduct a recording test to Metacog, further instructions in the QA Book. Do this for iPadOS + Safari and one other
random platform.
Conduct a memory test on the stand alone sim wrapper (rc.1).
Test one platform combination with ?phetioDebug=true on the Studio and State wrapper.
If Pan/Zoom is supported, make sure that it works when set with PhET-iO State.
Test that the sim works offline:

Click the link to the phet-io zip file (at top of issue) to download the zip file.
Unzip it to a spot locally.
Open index.html by double clicking it on your desktop or in a Finder-view.
It should look like the standalone version of the sim in PhET-iO brand.

Accessibility features

What to Test

Specific instructions can be found above.

Make sure the accessibility (a11y) feature that is being tested doesn't negatively affect the sim in any way. Here is
a list of features that may be supported in this test:

Alternative Input
Interactive Description
Sound and Sonification
Pan and Zoom
Mobile Description
Voicing

Test all possible forms of input.

Test all mouse/trackpad inputs.
Test all touchscreen inputs.
Test all keyboard navigation inputs (if applicable).
Test all forms of input with a screen reader (if applicable).

If this sim is not in this list or up to date there, make an
issue in website to ask if PhET research page links need updating. Please
assign to @terracoda and @emily-phet.

Screen Readers
This sim may support screen readers. If you are unfamiliar with screen readers, please ask Katie to introduce you to
screen readers. If you simply need a refresher on screen readers, please consult the
QA Book, which should have all of the information
you need as well as a link to a screen reader tutorial made by Jesse. Otherwise, look over the a11y view before opening
the simulation. Once you've done that, open the simulation and make sure alerts and descriptions work as intended.

Platforms and Screen Readers to Be Tested

Windows 10 + Latest Chrome + Latest JAWS
Windows 10 + Latest Firefox + Latest NVDA
macOS + Safari + VoiceOver
iOS + Safari + VoiceOver (only if specified in testing issue)

Critical Screen Reader Information
We are tracking known screen reader bugs in
here. If you find a
screen reader bug, please check it against this list.

Keyboard Navigation
This sim supports keyboard navigation. Please make sure it works as intended on all platforms by itself and with a
screen reader.
Magnification
This sim supports magnification with pinch and drag gestures on touch screens, keyboard shortcuts, and mouse/wheel controls. Please test magnfication and make sure
it is working as intended and well with the use cases of the simulation. Due to the way screen readers handle user input, magnification is NOT expected to
work while using a screen reader so there is no need to test this case.

FAQs for QA Members

There are multiple tests in this issue... Which test should I do first?
Test in order! Test the first thing first, the second thing second, and so on.

How should I format my issue?

We typically assign the developer who opened the issue in the QA repository.

My question isn't in here... What should I do?
You should:

Consult the QA Book.
Google it.
Ask Katie.
Ask a developer.
Google it again.
Cry.

QA is done