std/path: make globMatch work with @nogc

@jmdavis Brought this to my attention. I think this code points out a fundamental flaw in how we use @nogc both in Phobos and in general library code. Specifically, this code is technically correct. It is @nogc insofar as the GC is no longer called, but if somebody were to ever use this code in their @nogc hot-path code they'd probably be surprised to find out that it's allocating underneath them.
This is because in D, @nogc has two meanings. The first is the technical mean of "No GC Allocations". The second is community-semantic meaning of "no allocations". So in a way, this code is fibbing about what it does. It is technically correct, but for most users, they will see the @nogc and assume that the implementation does not allocate at all, which is incorrect, and they will only be able to determine the truth by actually reading the code, which of course, nobody does.
Of course, we can always fall-back to "you should just know that @nogc doesn't mean no allocations" but this is poor library design as this would require that we document all allocations regardless of allocation method and then tell everybody to RTFM. Furthermore, this code does not meet even the Phobos 3 standards for @nogc code. All @nogc code in Phobos 3 that requires allocation must request a buffer from the user (or an allocator, once we get those).
The overarching problem here is that @nogc has two different meanings that forces us to carefully consider our design choices. The design standards of Phobos 3 are intended to enforce a design where all @nogc methods are explicit about their allocation patterns in the call-site, which means that user is always aware of what they are doing. Given the present situation with @nogc this is likely the only realistic path forward for our @nogc library support.
As this code fails to meet the Phobos 3 design standards I would request that you either redesign this method to meet them or consider providing an alternative implementation via overload.

Calling a function allocates memory (on the stack).
I think you would need to define what "allocate memory" means more precisely before beginning to discuss whether @nogc allows it or not.

The first is the technical meaning of "No GC Allocations".

Actually that is not quite correct. @nogc means "does not use the GC", which includes other operations beyond allocating GC-owned memory. The point is that @nogc code can be linked into a program which does not include a garbage collector. Note that this is by itself useful in its own way, but quite far away from any perceived meaning of "does not allocate memory".

Calling a function allocates memory (on the stack).
I think you would need to define what "allocate memory" means more precisely before beginning to discuss whether @nogc allows it or not.

That's a valid point. We should try to be precise with what we mean. However, I think that it's pretty clear that he meant heap allocations, which is almost always what people mean when talking about allocations. Stack allocations happen all over the place whether you like it or not and aren't avoidable to the point that there's really no point in even discussing them with regards to APIs. The fact that they happen is a given, and they're a non-issue unless you start doing something like creating huge static arrays or run attempt really deep / infinite recursion.

The first is the technical meaning of "No GC Allocations".

Actually that is not quite correct. @nogc means "does not use the GC", which includes other operations beyond allocating GC-owned memory. The point is that @nogc code can be linked into a program which does not include a garbage collector. Note that this is by itself useful in its own way, but quite far away from any perceived meaning of "does not allocate memory".

From the standpoint of actually using @nogc, using it to determine whether you can avoid linking against the GC code is pretty pointless. You'll get that from the linker as soon as you try to link, and anyone looking to avoid the GC to that degree simply won't link in druntime. The whole point of @nogc is to be able to say that no GC allocations occur and that a GC collection can't be triggered, which enables you to reliably use that function in contexts where you can't afford for heap allocations to occur or for a GC collection to trigger. That's done by saying that no GC functions can be called, and in turn, that does technically mean that it will indicate that that function doesn't need symbols from the GC, but if that's your goal, @nogc is completely unnecessary.

The second is a community-semantic meaning of "no allocations".

No, it's conceptually wrong and I don't think the community follows that either. The whole reason for GC being costly is because of the entire mechanism of tracking the memory, not the allocation mechanism alone. The same for nothrow but with the argument that the exception unwinding code is expensive.

So in one sense, this code is fibbing about what it does. It is technically correct, but for most users, they will see the @nogc and assume that the implementation does not allocate at all

I think those are mostly your assumptions, at least in current Druntime/Phobos. I can prove that with a simple grep in the code. See https://github.com/dlang/phobos/blob/59b371567b65e331b4d219e35c7b4da0540a5970/std/algorithm/sorting.d#L3188 . I would accept arguing this should be documented, because documentation is where you clear out assumptions made by the users.
Take another example, where user assumes X or Y method is pretty fast, but turns out the method does a syscall in the background? What about a specific syscall that is potentially more expensive to the user? I think its not sensible to assume that the method does or does not a syscall nor a specific one, unless its documented properly, and I guess the same goes with allocations and other specific behaviour.
Would be cool to have rules for this, like enforce documenting it? Yes, do we have it currently? No. Can we still document it, yes.

Of course, we can always fall-back to "you should just know that @nogc doesn't mean no allocations"

It's literally the meaning of @nogc, usage of GC and nothing about "no allocations". Would you say, with your last argument, then, that mmap being @nogc it doesn't "allocate" at all? No, of course not. Allocation is also a very broad term. It vastly depends on your environment. Like @CyberShadow mentioned, isn't calling every function that uses stack, allocating?

All @nogc code in Phobos 3 that requires allocation must request a buffer from the user (or an allocator, once we get those).

While I agree with the allocations (we need to redesign the current Phobos allocators tho), why you bringing this PR, which changes a Phobos v2 implementation?

The overarching problem here is that @nogc has two different meanings that forces us to carefully consider our design choices

Sorry, but you seem to fabricate the second meaning. There's only one meaning of the @nogc attribute specified in the language documentation. I can see your point but this is not the current situation, at all.

As this code fails to meet the Phobos 3 design standards I would request that you either redesign this method to meet them or consider providing an alternative implementation via overload.

Why are we trying to make Phobos v2 the Phobos v3 already? We can clearly see this has room for debate, but I don't understand why here. The effort here is to avoid calling a GC collection for such trivial method. I could rework the method to not even use any sort of allocation, which, btw, I think that is the most sensible way, but that's beyond this change. This change itself is an improvement over the previous situation.

I think that we should stop considering that @nogc code is synonymous to 'extremely fast code' (and randomly add restrictions like "no allocations" to its meaning on that basis). You can be @nogc and still have expensive calls to other @nogc functions. @nogc functions simply mean that this function does not allocate using the gc (which by the way, is not that expensive in most situations), however, other functions that call the @nogc function can still use the gc. I don't think that there is a guarantee that @nogc guards you against a "stop the world" event if functions up the call stack have allocated gc memory (but I might be wrong). Anyway, my point is that if performance is what you are interested in, then the best way to do it is to start by using the gc, profile and see where the bottlenecks are. It might be that the gc is not where the problem is. This @nogc misconception only propagates gcphobia, when in fact a better design would be to have the possibility of selecting from a wide range of available gc's, including custom ones. I suspect that that will make gc more attractive for more people. And if your mind is set to not use the gc then you might as well compile with -betterC now that most hooks are templated.

I think that we should stop considering that https://github.com/nogc code is synonymous to 'extremely fast code' (and randomly add restrictions like "no allocations" to its meaning on that basis).

Thank you for raising this. I think this is not even consensual among the community either. And I'm pretty sure, we, at Weka, doesn't consider @nogc to be "the very fast implementation" of a GC alternative implementation, but rather an implementation with some considerations regarding runtime behaviour, because, honestly, it doesn't even make any sense to say that.
The boundaries are so vast that we would have to imply an extensive list of rules. Just by saying that syscall's can be worse, tells a lot about the above argument.

To complete some of the Weka concerns on Phobos:
@LightBender There's implementations that are supposed to be @nogc, but we don't use from Phobos, e.g. filter and a lot of range primitives. We have pretty much our own Phobos implementation because:

range API is fundamentally broken and potentially expensive due to exponential inlining cost. Modern languages implement a next that does have all three calls into one. This could be an alternative solution that ranges optionally implement.
std.algorithm most of the times creates a closure when its not needed, that's mainly why we have our own filter,etc ...
a lot of GC usage
a lot of enforce usage that implies throw and again, the GC too
auto decoding is an unfortunate feature too

(which by the way, is not that expensive in most situations)

In our case, using GC, regardless of being a small or large allocation, its potentially costly. I think with the refactor presented in dconf of the GC, this might be way better, but, mainly:

it may trigger an allocation, but if not
it may trigger a collection, but if not
it may trigger a page fault, but if not
it may trigger some syscall

Other reasons to avoid the current GC implementation and some Phobos implementations, for very specific reasons for us, like, but not limited, usage of huge pages, we also need to forbid all kinds of voluntary context switch when possible or usage of fork syscall, which, btw, its on a lot of std.process (which I need to make a PR to use vfork or posix_spawn).
Allocations via malloc are much more different and controllable. It may do a better use of a slab allocator and the fact that you can simply replace it with a drop-in implementation, is far better than the Druntime GC, which assumes a lot of things (which btw, I also suggest us standardise these assumptions), and therefore, not really replaceable.
All of those, in a hot path is costly, for some more than others.

@RazvanN7 Nominally, I agree. However, we have a perception problem with nogc. But this is not the only, or even the most significant argument against replacing the GC with malloc in Phobos. See below.
@ljmf00-wekaio Now lets address your concerns.
First. If a non-allocating implementation exists, why not use that?
Second. The reason to do this to Phobos 3 standards is simple. We're going to have to do it eventually when we pull this code into V3. So why not do it now? This code hits a key V3 design standard on allocations and there is no harm in either rewriting it to not allocate or adding an overload that takes in a buffer per the V3 standard.
Lastly. Consider that the new GC is going to hijack malloc/free so that it can track malloc'd memory in the C standard library. Therefore calling malloc actually will be the same as calling new. This means that under the new GC @nogc is a literal lie. More specifically @nogc will code will silently exhibit GC behaviors. The basic problem here is that "@nogc only means the GC" makes an assumption that is not guaranteed to be true. And in fact anybody can hijack malloc/free and do things beyond just allocating.
This is why the Phobos V3 design standard is "no/reduced allocations" not "@nogc". Allocation/deallocation runtime performance is in fact undefined from the perspective of the standard library.
Honestly, given what is coming with the new GC, and the idea of hijacking malloc/free in general, I think the value of @nogc is extremely limited, to the point where I would advocate removing it from the language entirely. And we're already talking about weakening it to allow allocations when throwing.
As far as I know, Weka likely won't care about any of this, because IIRC you guys do other things and don't use the standard GC. Therefore, while this change actually does deliver what you want, in the general case it, in fact, will do something else entirely. Our job with Phobos is to design to that general case.

to the point where I would advocate removing it from the language entirely

Music to my ears.

First. If a non-allocating implementation exists, why not use that?

I'm not against using a non-allocating approach, simply would require a bigger refactor that I'm not addressing on this PR. Could be done later, but should this improvement be a blocker? I don't see a reason.

Consider that the new GC is going to hijack malloc/free so that it can track malloc'd memory in the C standard library.
[...]
Honestly, given what is coming with the new GC, and the idea of hijacking malloc/free in general, I think the value of @nogc is extremely limited, to the point where I would advocate removing it from the language entirely. And we're already talking about weakening it to allow allocations when throwing.

This is not what I heard of it and there's concerns doing it, a lot of concerns frankly. Replacing the GC, I agree 100%, hijacking malloc/free, no. Would malloc trigger an implicit collection? This would be a disaster, IMO. GC would still collect and therefore would still stop the world, but in a way better implementation that triggers less of them. I'm afraid of this approach.
GC is very useful, and to be clear, I don't advocate for GC-fobia, but I think you underestimate the warm that GC causes to performant applications.

As far as I know, Weka likely won't care about any of this, because IIRC you guys do other things and don't use the standard GC.

We use the standard GC, and we planned to fork the GC for reasons that the new GC implementation is going to address, in most cases, observability/logging/tracing.

First. If a non-allocating implementation exists, why not use that?

I'm not against using a non-allocating approach, simply would require a bigger refactor that I'm not addressing on this PR. Could be done later, but should this improvement be a blocker? I don't see a reason.

Consider that the new GC is going to hijack malloc/free so that it can track malloc'd memory in the C standard library.
[...]
Honestly, given what is coming with the new GC, and the idea of hijacking malloc/free in general, I think the value of @nogc is extremely limited, to the point where I would advocate removing it from the language entirely. And we're already talking about weakening it to allow allocations when throwing.

This is not what I heard of it and there's concerns doing it, a lot of concerns frankly. Replacing the GC, I agree 100%, hijacking malloc/free, no. Would malloc trigger an implicit collection? This would be a disaster, IMO. GC would still collect and therefore would still stop the world, but in a way better implementation that triggers less of them. I'm afraid of this approach.

@ljmf00-wekaio @ljmf00
According to @deadalnix the new GC does not currently collect on malloc, but eventually it will. Presumably to improve the GC's handling of memory in applications that use C libraries. Therefore we cannot absolutely rely on malloc being non-collecting in the future.
While I am not inherently against merging this. We are creating another future piece of lint for us to keep track of for Phobos 3. But I do agree that the future GC behavior creates a bigger discussion around the entire concept of @nogc than can be addressed here.

This PR uses malloc/free in similar ways to other places in druntime and phobos.
If there was a problem with this PR, we would have problems all over the place.
Furthermore, any overriding of malloc will likely be both opt-in and POSIX only.
Therefore I'm in favor of this PR, and I do not understand why we have talked so much about this.

According to @deadalnix the new GC does not currently collect on malloc, but eventually it will.

Please don't. We really need to think about the consequences of that. I'm sure this is not one-sided by me, and I got to say, if that is assumed to be a default from now on, it would be catastrophic for so many high-performance programs, Weka very much included. You are underestimating what calling a/several syscalls or blocking all threads during important low-latency tasks really means. Allocations (e.g. malloc) alone doesn't do that often (in some applications not even close as often as the GC). Even if we think we could do a really great job on re-implementing the GC, the fundamental design principles doesn't let us reach nearly the same performance as, for example, Go GC (which is one of the top of the line GCs out there for a system language).
There's a reason for why people choose jemalloc, tcmalloc, mimalloc, etc... over the generic libc implementation. Saying, on top, @nogc will be eventually removed because of that decision is a very bold claim too.

Furthermore, any overriding of malloc will likely be both opt-in and POSIX only.

Being opt-in sounds better.

According to @deadalnix the new GC does not currently collect on malloc, but eventually it will. Presumably to improve the GC's handling of memory in applications that use C libraries. Therefore we cannot absolutely rely on malloc being non-collecting in the future.

I have (privately) given the feedback that this is never going to work. Our testing is showing signs that this is indeed problematic (because doing so requires knowing about all threads in the programs, so you also have to hijack pthread_create, which is its own can of worms). I would not make any decision based on this. However, I would love for scope array allocations to be @nogc, which we can do by lowering them to alloca.

hijack pthread_create, which is its own can of worms)

Right, I didn't thought of that, it can get worse easily if pthread_create on some linked code is hidden or putting it in very simple terms, people just using the clone syscall directly with sharing address space flag can cause problems. I think there's a lot of cans of worms there to open. It's simply not feasible to implement a drop-in replacement of malloc with the thread locking mechanism the GC has. And regardless, its catastrophic by the things I mentioned above.

scope array allocations to be @nogc, which we can do by lowering them to alloca.

LDC already sort of does that. https://github.com/ldc-developers/ldc/blob/master/gen/passes/GarbageCollect2Stack.cpp

LDC already sort of does that.

I'm aware, but it's an optimization pass, not something the frontend is aware of, and not something that can be enforced AFAIK.


Add Gradle script enabling new Javadoc tags

This PR resolves issue https://github.com/SpineEventEngine/config/issues/14.
Now the new Javadoc tags - @apiNote, @implSpec and @implNote - can be enabled in the build using:
apply from: deps.scripts.addJavadocTags

@armiol PTAL.


added validation for connection name

Connection name validation to check if it contains "-" and gives an error if yes

let's park this one for now.


fix propTypes issue

PropTypes no longer supported by react

@YuhanLee This needs an explicit dependency to the prop-types package in package.json. I can merge it as soon as you amend your commit. Thanks! 🙌

I have added the explicit dependency. Thank you !

@YuhanLee Thanks!


Enhancement of Patch Handling and PR Processing

PR Type:
Enhancement

PR Description:
This PR introduces several enhancements to the PR Agent:

Added an EDIT_TYPE enum to the git_provider module to distinguish between different types of file edits.
Updated handle_patch_deletions function in git_patch_processing.py to consider the EDIT_TYPE of the file.
Modified the get_pr_diff and pr_generate_compressed_diff functions in pr_processing.py to handle added files separately from modified files.
Updated the get_diff_files method in bitbucket_provider.py to assign the EDIT_TYPE to the FilePatchInfo object.
Adjusted the token clipping in get_pr_description method in git_provider.py to use a separate configuration for description tokens.

PR Main Files Walkthrough:
 files:

pr_agent/algo/git_patch_processing.py: The handle_patch_deletions function now takes an additional parameter edit_type to handle file deletions differently based on the type of edit.
pr_agent/algo/pr_processing.py: The get_pr_diff and pr_generate_compressed_diff functions have been updated to handle added files separately from modified files. Added files are now listed separately in the final diff.
pr_agent/git_providers/bitbucket_provider.py: The get_diff_files method now assigns the EDIT_TYPE to the FilePatchInfo object based on the status of the diff.
pr_agent/git_providers/git_provider.py: The get_pr_description method now uses a separate configuration for description tokens, allowing for more granular control over token clipping.

/describe

Preparing review...

Incremental PR Review

⏮ Review for commits since previous PR-Agent review: Starting from commit https://github.com/Codium-ai/pr-agent/pull/439/commits/c5bc4b44ff24858382fd98754868ac4fc9561128
(fix added files)

PR Analysis

🎯 Main theme: Enhancement of Patch Handling and PR Processing
📝 PR summary: This PR introduces several enhancements to the PR Agent, including the addition of an EDIT_TYPE enum to distinguish between different types of file edits, updates to various functions to handle added files separately from modified files, and adjustments to token clipping in the get_pr_description method.
📌 Type of PR: Enhancement
🧪 Relevant tests added: No

Incremental PR Review

⏮ Review for commits since previous PR-Agent review: Starting from commit https://github.com/Codium-ai/pr-agent/pull/439/commits/None

PR Analysis

🎯 Main theme: Enhancement of Patch Handling and PR Processing
📝 PR summary: This PR introduces several enhancements to the PR Agent, including the addition of an EDIT_TYPE enum to distinguish between different types of file edits, updates to the handle_patch_deletions function to consider the EDIT_TYPE of the file, and modifications to the get_pr_diff and pr_generate_compressed_diff functions to handle added files separately from modified files. It also updates the get_diff_files method to assign the EDIT_TYPE to the FilePatchInfo object and adjusts the token clipping in the get_pr_description method.
📌 Type of PR: Enhancement
🧪 Relevant tests added: No
⏱ Estimated effort to review [1-5]: 4, due to the multiple changes across different files and the introduction of new logic related to handling different types of file edits.
🔒 Security concerns: No security concerns found

PR Feedback

💡 General suggestions: The PR introduces a significant enhancement to the PR Agent by handling different types of file edits separately. This is a good practice as it allows for more granular control and better handling of different scenarios. However, it would be beneficial to ensure that all possible edge cases are handled correctly, especially when dealing with different types of file edits. It would also be helpful to add tests to verify the new functionality and ensure that it works as expected.

 🤖 Code feedback:

relevant file: pr_agent/algo/git_patch_processing.py
suggestion: Consider handling the case where edit_type is EDIT_TYPE.UNKNOWN. Currently, if edit_type is EDIT_TYPE.UNKNOWN, the code behaves as if the file was not deleted, which might not be the correct behavior. [important]
relevant line: if not new_file_content_str and edit_type != EDIT_TYPE.ADDED:

relevant file: pr_agent/algo/pr_processing.py
suggestion: It seems like the pr_generate_compressed_diff function now returns an additional value added_file_names. Ensure that all calls to this function are updated to handle this additional return value. [important]
relevant line: [patches_compressed, modified_file_names, deleted_file_names, added_file_names = ](https://github.com/Codium-ai/pr-agent/pull/439/files#diff-c2d64de584ef3cb05c9007d72137f6c7ce04c9bc23ce1931760af3a568edb04eR73)

relevant file: pr_agent/git_providers/bitbucket_provider.py
suggestion: The get_diff_files method now assigns the EDIT_TYPE to the FilePatchInfo object based on the status of the diff. It would be good to handle the case where the status of the diff is not one of the expected values. [medium]
relevant line: if diff.data['status'] == 'added':

relevant file: pr_agent/git_providers/github_provider.py
suggestion: Similar to the previous suggestion, ensure that all possible statuses are handled in the get_diff_files method. [medium]
relevant line: if file.status == 'added':

Preparing PR description...

Preparing PR description...

Preparing review...

Preparing review...


[T3A4][T17-A3]Sin Yu Fan

Check if file exists prior to executing command.
Print message to user if file is missing.
Create file.

@howitzerg Some comments added. Please close the PR after reading comments. You may wish to read and attempt the activity again. This is because the activity is about handling exceptions, but in your submission, you did not make use of any exceptions. You can look at https://github.com/nus-cs2103-AY1617S1/addressbook-level2/pull/273 as reference.


<IP_ADDRESS>

Sorry about the indent changes, it makes the diff very hard to read.
TLDR:

Added meshPolyRenderer.js that draws the mesh polygon (in the same way that C3 draws its own mesh poly)
Added properties for mesh poly
Fixed bug introduced by C3's module script changes. Basically, scripts are no longer executed in the global scope and so variables need to be explicitely taken or put on the global scope through the "self" keyword, namely all your classes and the "C3" object.
Another change I made was by passing C3 to the external scripts as a constructor param because it seems like C3 isn't defined on these script's global scope.
Bumped version number to <IP_ADDRESS>

@skymen Thanks for the PR! I'll check this out after work today
I don't have time to test this on my own. So if it's good on your end, then that's good enough for me :+1:.
Also, I'd like to add you as a contributor in the addon page if you're cool with that, what is your construct username so I can @ you?

This PR has been merged. I bunded the addon and published it to the addon store. https://www.construct.net/en/make-games/addons/370/renderbug/versions
Would you be willing to verify that everything is in working order? Thanks again

Also, I'd like to add you as a contributor in the addon page if you're cool with that, what is your construct username so I can @ you?
Yeah sure, my Construct username is @skymen :)

I'll do a round of testing. I've been using this in a project of mine so I'll see pretty quickly if something broke 👍

Did a few quick tests and couldn't find any issues with it so it seems all good to me


Publishing packages doesn't expand them as did "nuget init -expand"

I'm using new repository folder structure (described here ). I converted the old structure to a new one using the nuget init command with the -Expand switch
Publish package is working from nuget push and code (Nuget.Core).
The problem that the -Expand command extracted existing packages, while publish doesn't. I would like to use expanded files in some scenarios so interesting if there is an issue with Nuget.Server, which handles publish requests?
Thanks

NuGet.exe push <package> -Source <server URL>

Sorry, but didn't understand you comment. I mean why do you think I didn't tried it before opened this issue? An if you closed this issue - this means it's solved?

Are yoy pusing to the server URL or to the server's package path? (http(s) or the full path to the server)?

To URL, so it will pass the Nuget server app. The direct access to packages folder is closed

Then the server should expand. What version of the server are you using?

NuGet.Server: 2.10.3

By the say, what is the purpose of that expansion? Can't find any documentation.

Here's the why: http://blog.nuget.org/20160113/Accelerate-your-NuGet.Server.html
Note that the server only expands the .nuspec and package hash, not the actual files in the package (as the server doesn't expose those directly anyway)
Are you looking into expanding all files?

I expecting it to expand the lib folder as well as done by the nuget init -expand. .nuspec and package hash expanded without -expand option. NuGet.Server do it right. Sorry for being unclear. So the only thing differ nuget init -expand from NuGet.Server is lib folder (in my case).
nuget.exe help says:
Expand - If provided, all the files in the package(s) are added to destination package source.

Got it :) NuGet.Server does not support this further expansion, it just needs the .nuspec and hash.

Ok, if it's by design - no problems.
Thanks


Transparency only when one window is open ?

Is your feature request related to a problem? Please describe.
No
Why does polybar need this feature?
It's more beautiful and sweet
Describe the solution you'd like
I'd like to have the transparency only when one window is open because that's weird to see my background above the window and not around. Also I'd like to have trnasparency when no window are open because it's beautiful and when two windows or more are open because I have smart_gaps between the windows (on i3)
Screenshot
I edited the config between the screenshot

TBH I don't see this feature being regularly used. Couldn't you write a script or program that draws a window (blank bar or plain X11 window) the same size as the bar behind it, or even have two different wallpapers?

Thank for your solutions, I'll try this but I think later it can be very cool if you add the functionality. And it's regularly used on Gnome, the default bar is transparent on the desktop (I think 50% of opacity) and when there is a full-screen windows the bar is Grey like the title bar.
In a nutshell, I think the best is add this functionality to polybar


chore(TDI-39964): common jars alignment

What is the current behavior? (You can also link to an open issue here)
https://jira.talendforge.org/browse/TDI-39964
What is the new behavior?
Please check if the PR fulfills these requirements

[X] The commit message follows Talend standard
[X] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features) ?
[ ] The code coverage on new code >75%
[ ] The new code does not introduce new technical issues (sonar / eslint)

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[X] Refactoring (no functional changes, no api changes)
[ ] Build / CI related changes
[ ] Other... Please describe:

Does this PR introduce a breaking change?

[ ] Yes
[X] No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...


Support nested_attributes

To add values to the nested attributes for some of my Models, I have to do the following
   factory(:avatar) do
     icon 'smiling'
   end

   factory(:user) do
     avatar_attributes { attributes_for(:avatar) }
   end

It'd be wonderful if I could just do:
  factory(:user) do
    avatar_attributes
  end

or
  factory(:user) do
    nested_attributes :avatar, :factory => :awesome_avatar, {:icon => 'override_awesome_avatar'}
  end

What do you thing about it? By now I've resolved for has_one association but not for has_many associations maybe related with #202

I agree with @pacoguzman and even further, if you specify :attributes_for for a factory and there is an association or a lazily loaded factory (eg { FactoryGirl.build :foo }), there should be an option to have it override it so that it too spits out attributes and you end up with a hash of hashes.
This is a very similar request to this issue: #191 and #64 where the creation type of child factories needs to be inherited from the parent factory.  Due to this, it may be an indication that a more elegant solution may be able to be found rather than hacking something together for each use case.

Although to be fair @pacoguzman, I think that association is still the proper DSL to use in your case.  It's just that if you specify attributes_for :user, it should be smart enough to give you attributes back for the avatar as well as avatar_attributes.  If you did not specify association it should be assumed that you just want a key with the name of the factory (in this case avatar).

@jfelchener I'm not following you. I want to build objects with factory girl as I build in my controllers/forms, setting nested_attributes is not exactly the same that associate or build an associaton for the object

Ok.  Try setting accepts_nested_attributes for an attribute you don't have an association for and let me know what happens. ;)

I don't know, but maybe throws and exception IMHO it's better when you know how your models are defined. But anyway I've implemented this and I've learn how factory girl works so it's worth it

thanks @pacoguzman , was stuck on this for a day and found this (http://stackoverflow.com/questions/32450502/rspec-factory-girl-build-with-has-many/32468672).


Unable to import

When I try to import this using the instructions on the Readme, I get the following error:

macOS Mojave 10.14
Terminal Version 2.9 (421)

Looking into this issue further, it seems that it might be a problem with macOS, because if I get this error first:

and then I go into System Preferences to "Open anyway":

then Terminal complains that the file is corrupt and can't be installed. However, if I do a direct Download of the file and save it into a new file, then there is no problem.
I just did this to get around the security warning:
$ curl https://raw.githubusercontent.com/sindresorhus/terminal-snazzy/master/Snazzy.terminal > Snazzy.terminal

and double-clicked on the curled Snazzy.terminal and it worked. So I am not sure if the macOS security settings are somehow messing with the file?


Add systemd service information

Add install instructions to create systemd service so that acme-dns can be automatically run on boot and log tracked by systemd.

Coverage remained the same at 92.244% when pulling 45bc0a37cebb557723c617352965974d15b06753 on gabe565:service-documentation into 3006cb712bfb45155d554c4e610b594021c1d891 on joohoi:master.

I agree with the installer script. For now I have fixed the changes in the documentation, but eventually an installer would help to copy a lot of these files where they belong and simplify this documentation.

LGTM! Thanks for the PR!


Prehit does not work in combat

still have to wait for swing first, then it hits, instead of hitting then delaying the swing.
CombatFlags=01|040|0100
CombatDamageEra=0
CombatHitChanceEra=0
SpeedScaleFactor=15000

Actually you need to wait only 1 second, which is the minimum possible duration for the attack animation, and it's intended to work like that, you don't wait the recoil time but only the swing one

Nolok I'm not sure if you saw the discord chat, me and Drk tested it its not working correctly for either of us.  :O the initial attack takes longer than 1 second, then with COMBAT_RECOIL_NORANGE  that doesnt seem to work either, it's not starting the next swing until you come back in range, then you can leave again and come back to hit.
typically on OSI the first strike was instant, not a 1 second delay, i just tested that to confirm a few days ago.

somewhat not related at all but more bothersome to me personally, with COMBAT_RECOIL_NORANG it does the first hit even if you are in range or not.

it seems like if we are able to get the firsthit_instant to work, then people will be able to emulate the OSI combat system pretty closely. right now the first hit is not instant with that flag enabled.

looks like enabling prehit, and firsthit_instant (which should replicate OSI), firsthit never seems to work. but prehit is working fine.


Is it possible to add a compiled dist folder if people directly want to integrate the client in a frontend?

I'm not sure if this is best practice, but it might be helpful for those making static pages.

If I understand this correctly you would like to have a pre-built version of the JS available in an npm package.
Some libraries did do that in the past. But that turned out to be a bit cumbersome. For example scoping issues. Or versioning and integrity.
Having a pre-built package in a node_modules folder would still mean that you need to somehow copy that file into your project. Which is also effectively a build step. So in those cases you might as well use a minifiy/bundle step.
Have you considered to publish a minified version on a CDN on the releases?
i.e.: https://cdnjs.cloudflare.com/ajax/libs/weaviate-javascript-client/x.x.x/browser.min.js.

That's actually a good idea, thanks for sharing @StudioSpindle!

CC: @waldenn

@halvespereira – based on @StudioSpindle's suggestion. Can we -in Travis- generate a minified JS that can be added to the browser? I can push it to a location so that people can use it

@bobvanluijt I'm not sure how to achieve this in Travis. I think @etiennedi or @antas-marcin might be able to help better with this?

Hi @halvespereira – I think I can relatively easily help. But how could the dist folder be created?

Usually running npm run build generates the dist folder. But looking at the client repo, I noticed we are using rollup as part of the build process. I'm not too familiar with what that is or how it works. Whoever set up that build process might be able to help faster.

I hacked together a working bundled version using esbuild (should be possible with rollup too).
Code changes:
diff --git a/graphql/aggregator.js b/graphql/aggregator.js
index 1cff3e7..328a23a 100644
--- a/graphql/aggregator.js
+++ b/graphql/aggregator.js
@@ -99,4 +99,4 @@ export default class Aggregator {
   };
 }

-module.exports = Aggregator;
+//module.exports = Aggregator;
diff --git a/graphql/nearImage.js b/graphql/nearImage.js
index 3e1c969..28b2f58 100644
--- a/graphql/nearImage.js
+++ b/graphql/nearImage.js
@@ -1,4 +1,4 @@
-const fs = require('fs');
+//const fs = require('fs');

 export default class GraphQLNearImage {
   constructor(nearImageObj) {
diff --git a/index.js b/index.js
index 00654b6..0a8f749 100644
--- a/index.js
+++ b/index.js
@@ -44,5 +44,5 @@ const app = {
   KIND_ACTIONS,
 };

-export default app;
+//export default app;
 module.exports = app;

Then ran:
esbuild index.js --bundle --outfile=dist/lib.js
Then edited the file in "dist/lib.js" by adding "let weaviate = {};" at the top and "var weaviate = app;" at the bottom (before module.exports = app;).
Now the library seems to work, but I have no experience yet with Weaviate to really verify that (need to read the docs first).
I tried using it with this endpoint I saw in the online playground, but got CORS issues:
const client = weaviate.client({
scheme: 'http',
host: 'semantic-search-wikipedia-with-weaviate.api.vectors.network:8080',
});
"Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://semantic-search-wikipedia-with-weaviate.api.vectors.network:8080/v1/schema. (Reason: CORS request did not succeed). Status code: (null)."
Is there a way to get that working without a CORS proxy?

Thanks @waldenn –
@halvespereira can you look into this CORS issue, please? Might be nice to have an OOTB dist/ that people can use in the bowser directly

Thanks @waldenn –
@halvespereira can you look into this CORS issue, please? Might be nice to have an OOTB dist/ that people can use in the bowser directly

Don't have time now to look further into this right now, but maybe it was the http/https issue triggering CORS. Using https is pretty much required these days. When I get some time, I'll read up on the Weaviate docs, but is there are way to directly enter the GraphQL query into the weaviate JS client call, and not use the Weaviate function API?

@waldenn – not yet, @etiennedi is in the process of aligning all the clients.
@etiennedi – is there maybe a GH issue where @waldenn can subscribe to, to get updates on the status of the above?


Exercise 2 details

LGTM. @bocon13 as you noted it's important to add steps to test ping on mininet and specify that the topology define multiple subnets, one of which has multiple hosts (h1a, h1b, etc)

we also need to add solution dir.
I am working on putting ---- START/END SLUTION ---- to the IPv6 solution

@Yi-Tseng let's wait before modifying the code and moving files to the solution directory, that should be the last step. Let's add SOLUTION BEGIN/END markers, then we'll take out code and create the solution directory

ok, no problem


Swizzle URLSession Metrics

Hey! Got a bit of a feature request here 🤓
How would you feel about swizzling this request: urlSession(_:task:didFinishCollecting:)
This is a really interesting delegate call because it gives you access to the task metrics which includes how long every part of the request took including the DNS lookup, connection, and of course the server response time.

It also provides more information about the request itself including the remote server details, the protocol used and whether it was on a cellular network or not.
All of this data feels useful not only for Atlantis but Proxyman too as it provides a much richer understanding of the time taken to make requests.

Worth noting, some (most?) is only available on iOS 13+ (etc) - so should just be a nil object on incompatible versions rather than bumping the package's minimum up.

Thanks for opening this ticket @Sherlouk
I suppose that I should support your request soon and show the data in the Summary Tab on Proxyman macOS 😄

I will put this feature in the next release 🎉

Any news regarding that?

Hey @dnKaratzas, I haven't worked on it yet. Which timing elements that you concerned?


This is not helpful as you cannot move keys from one region to another.

[Enter feedback here]
The idea is to be able to move a keys from a keyvault from one region to another as per the title of this document.
However, there doesn't seem to be a way to move a KEY from one region to another as all my attempts at downloading/backup and uploading/restoring have failed.
https://learn.microsoft.com/en-us/azure/key-vault/general/move-region#:~:text=Using the backup and restore commands has two limitations%3A
az keyvault key backup --file hardhat-0.bin --name hardhat-0 --vault-name kv-eastus
az keyvault key restore --file hardhat-0.bin --vault-name kv-germany-west-central
Cannot decrypt backup blob.

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

ID: 87ac0a31-bed7-0ef8-54b3-d4c465ed33da
Version Independent ID: dd9dbc54-8c13-3cc1-e761-80123bcc2334
Content: Move a key vault to a different region - Azure Key Vault
Content Source: articles/key-vault/general/move-region.md
Service: key-vault
Sub-service: general
GitHub Login: @msmbaldwin
Microsoft Alias: mbaldwin

@rscottwatson
Thank you for bringing this to our attention.
I've delegated this to content author @msmbaldwin, who will review it and offer their insightful opinions.

@rscottwatson this different geography (US and Germany) you can only restore within same GEO. Key Vaults are protected with GEO boundary :
Design considerations
When you back up a key vault object, such as a secret, key, or certificate, the backup operation will download the object as an encrypted blob. This blob can't be decrypted outside of Azure. To get usable data from this blob, you must restore the blob into a key vault within the same Azure subscription and Azure geography.

#please-close


JIT layered on PIM

There should be some documentation / verbiage / links to information on minimal requirements when JIT is in use along with PIM.  Need to understand what must be activated to be able to perform a JIT activation

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

ID: 25f0758b-8274-d2b5-4702-2c7ca6b0b14a
Version Independent ID: d1215107-abf9-fefd-a63b-b20ec3f7f744
Content: Just-in-time virtual machine access in Azure Security Center
Content Source: articles/security-center/security-center-just-in-time.md
Service: security-center
GitHub Login: @memildin
Microsoft Alias: memildin

@Torxsmind
Thanks for your feedback! We will investigate and update as appropriate.

Hi @Torxsmind, thanks for the feedback and for taking the time to help us improve the docs.
What is the problem you're trying to solve?
The specific permissions required for JIT are documented in What permissions are needed to configure and use JIT?
What special requirements do you see from using PIM?

@memildin we have a locked down PIM setup along with JIT.  I am looking for documentation / guidance on the permissions needed to allow a developer to request JIT.  I believe the real issue comes to writing the subnet NSG or local NIC NSG.  This requires write access privileges.
According to the link you pasted:

On the scope of a subscription or resource group that is associated with the VM:
Microsoft.Security/locations/jitNetworkAccessPolicies/initiate/action

This is the only permission I see that would / might give the developer access.  I will try it to see what happens.
Overall going back to my initial comment he issue there should be some clarification / considerations when using JIT and .

@Torxsmind, thanks for the additional details.
As mentioned on that page, there's also a script for creating a least-privileged role for JIT access: Set-JitLeastPrivilegedRole script from the Security Center GitHub community pages.
I think your comment got cut at the end ("there should be some clarification / considerations when using JIT and .").

@Torxsmind have you had a chance to try JIT yet?
If it's okay with you, I'll close this issue and if you have further questions you can open a new one or reopen this one.
#please-close


Multipart array

When sending an array in a simple urlencoded POST, it gets converted into an object or an array of objects thanks to the qs lib used by the co-body lib.
Example:
 books[new-1][is_new]=1
 books[new-1][title]=test

Becomes:
 {
     "books": {
         "new-1": {
             "is_new": "1",
             "title":"test"
         }
     }
 }

But when the same field is sent in a multipart POST (e.g. with a picture). Then the formidable lib is used, and because this lib only processes the fields independantly, it doesn't do the conversion and we end up having the following:
 {
     "books[new-1][is_new]":"1",
     "books[new-1][title]":"test"
 }

A simple fix I have found is to ask qs to rebuild a query string and reparse it instantly like so:
qs.parse(qs.stringify(body.fields))

It probably is not the best solution but it does the job

Thanks, will look at this over the weekend.

any update on this?


Added documentation for iter types and structs

Added documentation for all iter traits and structs. Updated links to par_iter and par_iter_mut so they link directly to the methods (previously they linked to the trait page, not the specific method).

@cuviper I updated the docs to be more consistent. I removed the unused links and added in the suggestion to just implement IntoParallelIterator

Looks great, thanks!


3367 fix test case

Overview
This fixes the problematic test case which was failing in the case when users db exists.
Testing recommendations
Run make eunit apps=chttpd tests=all_test twice
Checklist

[x] Code is written and works correctly;
[x] Changes are covered by tests;
[ ] Documentation reflects the changes;

+1


Test Run error in jsh file for CS2030 Exercise #2: Cruise Loaders

I ran into an incorrect output in the jsh file for the exercise #2: Cruise Loaders. I've attached the incorrect output below.
My output is missing /open level4.jsh
What snippet of code do I need to fix it?

If you have defined a level4.jsh in your submissions, try resubmitting it. I've encountered this issue before, think its a bug.

If you have defined a level4.jsh in your submissions, try resubmitting it. I've encountered this issue before, think its a bug.

Thank you! re-submitting it helped!

You may want to put down your submission ID for that run here so that @danqiye1 can look it up :)


autoupdate hash url should be respected

I'm trying to allow autoupdate for a psmodule which is placed in the same repo as the bucket.
Since I can't use the buckets master.zip to verify the hash I'd like to hash against the psmodule manifest  (*.psd1). The hash url does not seem to be respected when used like this:
{
  "version": "1.2",
  "url": "https://github.com/krokofant/scoop-powershell-bucket/archive/master.zip",
  "hash": "f0ee361a0cc501dbc1932dec46ce25faaa7e53218efc52c68256379f9c285317",
  "extract_dir": "scoop-powershell-bucket-master/modules/PS-Install-If-Missing",
  "psmodule": {
    "name": "PS-Install-If-Missing"
  },
  "checkver": {
    "url": "https://raw.githubusercontent.com/krokofant/scoop-powershell-bucket/master/modules/PS-Install-If-Missing/PS-Install-If-Missing.psd1",
    "re": "ModuleVersion\\s+=\\s+'([\\d.]+)'"
  },
  "autoupdate": {
    "url": "https://github.com/krokofant/scoop-powershell-bucket/archive/master.zip",
    "hash": {
      "url": "https://raw.githubusercontent.com/krokofant/scoop-powershell-bucket/master/modules/PS-Install-If-Missing/PS-Install-If-Missing.psd1"
    }
  }
}

This seems to download the autoupdate.url and hash check against that instead of using autoupdate.hash.url. Using the psd1 file as the autoupdate url would overwrite the actual download url.
If autoupdate.hash.url is specificed, should that not be used?
Could not find hash in https://raw.githubusercontent.com/krokofant/scoop-powershell-bucket/master/modules/PS-Install-If-Missing/PS-Install-If-Missing.psd1
Downloading master.zip to compute hashes!
Loading master.zip from cache
Computed hash: 0e41359d359f2b87a522d48a54d88365542701044a37b845fcf7f8348289a9e4

I expect the fallback download option to download the autoupdate.hash.url and not the autoupdate.url.

https://github.com/lukesampson/scoop/wiki/App-Manifest-Autoupdate#add-hash-to-autoupdate
au.hash.url is URL template for downloading RDF/JSON files or extracting hashes.

@niheaven I see, what I interpreted from the docs was that the "mode": "download" would download au.hash.url instead of the au.url. The other au.hash.mode alternatives seem to process the au.hash.url.

mode:

extract: (default, can be omitted) download from url and find with the regex
rdf: extract from a RDF file. Example: imagemagick
json: extract from a JSON file. Example: openssl
download: (fallback) downloads the file and hash it locally

I know it sort of defeats the purpose of the hash if it could be generated and checked against an arbitrary source but it would be great if there was a way of calculating the hash from a path inside a zip so that one github repository could contain several apps/modules.

It could, by give seperate url in git repo, please check https://github.com/scoopinstaller/scoop-main/blob/master/bucket/psutils.json, and https://github.com/scoopinstaller/scoop-main/blob/master/bucket/ln.json, https://github.com/scoopinstaller/scoop-main/blob/master/bucket/sudo.json. ln, sudo and several other apps are in psutils and also as seperate apps.


Modify chain consensus parameters to be height aware

Opening early so you can see progress, but this is no further along than it happens to compile.

It's based on #1403, but they're split to make them a bit easier to review.

This is running through a further full resync after rebase, but I'm fairly happy with just merging this now if it's okay?


add revertable

While building a service around this library, i noticed that if the transaction is new, there's a revertable attribute, which it doesn't support.
Added it with default=False
Probably could also be added as a property (not sure):
@property
def revertable(self):
    return self.state == 'pending'

service_1   | [2022-05-15 08:31:27,404: ERROR/ForkPoolWorker-2] Task payments.tasks.load_revolut_transactions[bad11cfc-211b-4eae-96d4-27a5331c6d64] raised unexpected: ValueError("Excess keyword for <class 'revolut.Transaction'>: revertable = True")
service_1   | Traceback (most recent call last):
service_1   |   File "/usr/local/lib/python3.8/site-packages/celery/app/trace.py", line 451, in trace_task
service_1   |     R = retval = fun(*args, **kwargs)
service_1   |   File "/usr/local/lib/python3.8/site-packages/celery/app/trace.py", line 734, in __protected_call__
service_1   |     return self.run(*args, **kwargs)
service_1   |   File "/code/payments/tasks.py", line 145, in load_revolut_transactions
service_1   |     for transaction in client.transactions(from_date=from_date):
service_1   |   File "/usr/local/lib/python3.8/site-packages/revolut/__init__.py", line 158, in transactions
service_1   |     txn = Transaction(client=self, **txdat)
service_1   |   File "/usr/local/lib/python3.8/site-packages/revolut/__init__.py", line 476, in __init__
service_1   |     self._update(**kwargs)
service_1   |   File "/usr/local/lib/python3.8/site-packages/revolut/__init__.py", line 496, in _update
service_1   |     super(Transaction, self)._update(**kwargs)
service_1   |   File "/usr/local/lib/python3.8/site-packages/revolut/__init__.py", line 173, in _update
service_1   |     raise ValueError(
service_1   | ValueError: Excess keyword for <class 'revolut.Transaction'>: revertable = True

Thank you!

released as v0.8.6


sphinx documentation doesn't update new classes properly

Following issue might help us
#https://github.com/astropy/sphinx-automodapi/issues/17

The current problem is that the documentation for the class Audio2AFCSimulation is poor. It doesn't reflect the docstrings from the module.

Problem persists with latest iteration of code. For instance, the likelihoods attribute of the class KnownHazard here doesn't show up in doc.


Update Flight Hub to add IoT, ISO & SDK builds for the RS2 section

from https://github.com/MicrosoftDocs/windows-insider/pull/224
This PR adds:

Added all IoT, ISO & SDK builds that have went off with RS2 flighting.

EDIT: It seems like this change may have been reverted previously due to a conflict:
My RS2 change from PR was added in https://github.com/MicrosoftDocs/windows-insider/commit/103a90ff959426900456ddbd6a2077754688d130, was good until https://github.com/MicrosoftDocs/windows-insider/commit/c7b57d55eb0428ab528551a98cedf0a7cc3ca5a1, but got reverted in https://github.com/MicrosoftDocs/windows-insider/commit/27ec156dd1c7c9cbd9a160efe8be4d7d465dd00a after my RS1 PR got merged.

@nenonix @JohanFreelancer9 - kindly do the copy editor review for this PR. Thanks.

cc @JohanFreelancer9 the links don't seem to work when the locale is removed.

@lizap - PR is ready for final review.
Thank you.
cc: @kenwith

cc @lizap

adding @bleblancMSFT to the assignee as suggested by lizap to help review and merge this PR.
Thank you.
cc: @kenwith

cc @bleblancMSFT

@ItzLevvie - can you hold off going back and adding updates for prior releases like RS4-TH1 etc.? We're doing some updates and adjustments we want to ship and some of the items you're adding doesn't match up with our records. Also - we do not want to add Mobile builds to Flight Hub.

@bleblancMSFT Alright.

@ItzLevvie I added in the 19002 SDK release and latest 19H2 releases. Let me know if it looks right!

@bleblancMSFT Just a spelling mistake needs to be fixed. I have opened a PR over at https://github.com/MicrosoftDocs/windows-insider/pull/242. Other than that, it looks good :)

@ItzLevvie just approved and did the merge! THANK YOU!


Add dutch translation

I'd like to see a dutch translation. I can translate to dutch, because I'm a native speaker, but I don't know how to contribute to this project. Is there an easy way for me to contribute? If it isn't too much hassle, I'd like to know how to contribute to this project with a few basic instructions for somebody that has never worked with applications for MacOS. Otherwise, I guess a list of translated strings would do, don't you think?

Hi, Dutch would be fab.
An easy way is for you to take the attached file and edit it in a text editor.
Localizable.txt
Each line is a pair of strings. Leave the string on the left untouched, and edit the string on the right to be Dutch. Then just shoot it back to me.

I've incorporated this into a test build: https://github.com/pilotmoon/Scroll-Reverser/releases/tag/1.7.5-pre0

Awesome, thanks for the quick implementation and the merge. I love the app, by the way! Truly essential for users of multiple pointing devices with various scroll directions.


release notes: minor changes

MishaBoar was listed twice because of capitalization

Because this is just 1 line, I'm leaving this as a draft in case any of the following makes it in:

#3036
#3029

as these would change release notes.

Actually feel that we should credit reviewers too in the notes, so I added them after review. Used the following script to actually get reviewers programmatically (needs a token), and added those that weren't already listed:
TOKEN="<your_token_here>"
PRS=`curl -s "https://api.github.com/repos/dogecoin/dogecoin/pulls?base=1.14.6-dev&state=closed&per_page=100&page=1" | jq '.[].number' && curl -s "https://api.github.com/repos/dogecoin/dogecoin/pulls?base=1.14.6-dev&state=closed&per_page=100&page=2" | jq '.[].number'`
for pr in $PRS; do curl -H "Authorization: token $TOKEN" https://api.github.com/repos/dogecoin/dogecoin/pulls/${pr}/reviews |jq -r '.[].user.login'; sleep 1; done | sort | uniq

Addressed https://github.com/dogecoin/dogecoin/pull/3046#discussion_r922751079

added Portuguese translation and contributors from #3029

Added Bulgarian translation and contributors from #3036

I think that's all... ready for review!

Added second reviewer from #3036 to credits.

Per our urgent guidelines, merging now. Comments and issues and further PRs are welcome.


Object key type narrowing for a variable when using in operator

Suggestion

🔍 Search Terms

in operator, in keyword, object key type narrowing inferring
✅ Viability Checklist

My suggestion meets these guidelines:

[x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
[x] This wouldn't change the runtime behavior of existing JavaScript code
[x] This could be implemented without emitting different JS based on the types of the expressions
[?] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
[x] This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion
in operator can be used to narrow down object's union type, but it also could be useful to narrow down the key type as well
📃 Motivating Example

https://www.typescriptlang.org/play?#code/MYewdgzgLgBAtgQwA4wLwwN4Cga5gawFMBPAQQC4YBtAclJoF0AaHPI4gIUpo5qwF8sWAPTCYwBGBgAjQjEnEY0AE4BLMAHMsAG0KwIIOIQDKUNZrQwa7egG4hqgGYAKA0dPmNMdfGQBKbDwYURhCZWUQZQBCVlxEJCo3EzN1DQZ7WOCxYABXcMIwWAB3SPwECJywABN5atCADyRCYChCGtkACwQAN1VIzPjEw2TPeQgCEhBHGChiJqnfJAYBIA
const map = {
    keyA: ['A'],
    keyB: 'B'
}

// can be any string
let someString = 'keyA';

if(someString in map){
    // error!
    map[someString];

    // current workaround and expected behavior
    map[someString as keyof typeof map]
}

💻 Use Cases

Simplify code, remove unnecessary type casts

This already works provided someString is declared as a const variable.
https://www.typescriptlang.org/play?#code/MYewdgzgLgBAtgQwA4wLwwN4Cga5gawFMBPAQQC4YBtAclJoF0AaHPI4gIUpo5qwF8sWAPTCYwBGBgAjQjEnEY0AE4BLMAHMsoSLAgg4hAMpQ1mtDBrt6AbiGqAZgAp9hk2Y0x18ZAEpseDCiMITKyiDKAISsuIhIVK7GpuoaDHYxQWLAAK5hhGCwAO4R+Ajh2WAAJvJVIQAeSITAUITVsgAWCABuqhEZcQkGSR7yEAQkIA4wUMSNkz5IDAJAA
It doesn't work with let variables because we'd have to do additional (an expensive) analysis to prove the variable isn't mutated between the in check and the element access.

@ahejlsberg yes, the suggestion is specifically for let/var, not const.
Could you elaborate how would it be any different from already implemented behavior with object type narrowing and then mutating it?
Comparison:
https://www.typescriptlang.org/play?#code/DYUwLgBA9gXBDeBDOBGAvhAPggRnATGgFBEAUpAlBALwB8CRAlgGakDkibEjAdtBfCIRhEAPSiIPRACdpUAO4gAJkJFQSIsRIC2AVzCIwygDQQAzlG5g2ZiLp6MofRAHNEvVcMvVcMQgG4iYjQKEnFwkgBjJzNIbUQABxoGTWQIAAZjTwg8CBQgklBIMxhY6V4XQJJyCjpBbJZSW14IeISBbM1bcXMACyhdYCUckAgAaxAATyhmCDBJhJAZ1sSNTRFbHwAieV7DEAA3EGktrQhmRGBgWxxESLG5yy2yiq3s4IogA

Could you elaborate how would it be any different from already implemented behavior with object type narrowing and then mutating it?

The s in map construct only attempts to narrow map and has no effect on s. It wouldn't be sound to narrow s because map could well reference an object instance that has more properties than are known by the static type.

could well reference an object instance that has more properties than are known by the static type

But it doesn't seem to apply when the map is const. TS will prevent adding more properties as well, so it looks like if  s in map is truthy, then s must be a keyof map. Correct?
https://www.typescriptlang.org/play?#code/MYewdgzgLgBAtgQwA4wLwwN4Cga5ggLhgAYAaHPAIyIEYsBfLLRJAOmDRgCYBuJlzhmBEu9PlgA2AU1gQi0AE4BLMAHNxWABSaAlGgB8mJnhhKAZpoimw8ZDuwmTVgPTOYEABYgArhIAmMJRSMADWUgCeIGYwUOFIUlG2SMaOuFboAEQA7h4IUFIAblIKGTCuMGYIEhJWlAjAITEgMBmKKqoZFLiM9DpAA

@Lutymane Objects are structurally typed. This is legal:
const src = { a: 0, b: 1, c: true }
const map: { a: number, b: number } = src

@MartinJohns Oh, thank you for the example! Now it makes sense. So the difficulty here is tracking up the full set of keys for the map object comparing to its type definition, I see.
Although, I suppose it's technically possible to check if the const was defined by a literal and that the type was inferred from that literal or type was not overridden when assigning identifier, but I guess it's not that easy to implement?

I suppose it's technically possible

It's only possible in some narrow cases, but not generally. The object could be created from within a function, or it could even be created from an imported module where the implementation is simply not known at all.


Not able iterate through each struct elements in XML

Hello ,
I am trying to iterate through elements mentioned in Struct
e.g. struct  < name:string,result:string, marks:int, age:int >  from XML. (I want to access each element in that struct one by one)
How can I do that ? May I know "column.xpath.result" syntax for that to iterate through each element?
OR Do I need to use other Data type like struct ?

I am not entirely sure I understand the question. Please open a separate issue and provide some simple XML example and the desired  Hive table schema.


Engine API spec improvement proposal

Main ideas behind this proposal were brought up during the Engine API session at the Devcon R&D workshop. Thanks everyone attended for fruitful and engaged conversation. Special thanks to @protolambda for taking notes.
Upcoming HFs introduce changes to existing Engine API data structures and method semantics, and also may require new methods to be added. Outside of HFs scope, there are a number of proposals to add auxiliary methods for optimisation and usability purposes (https://github.com/ethereum/execution-apis/pull/218, https://github.com/ethereum/execution-apis/pull/318). In some cases auxiliary methods may be optional, i.e. EL may not support such a method but if supported it can be utilized by CL.
On the other side, we need a convenient way of deprecating unused or redundant methods (e.g.  engine_exchangeTransitionConfigurationV1).
Capability of introducing changes to the Engine API without coordinated upgrades between the layers is an important property of proposed design.
Method and structure versioning
This proposal doesn't affect requirements described in the Versioning section of the original specification. Any changes to a method behaviour or its parameter set, likewise, changes to the fields of a data structure should be signified by bumping its version. A version is reflected in the name of each method and structure in a VX suffix, where X is a number of the version.
This approach makes specification clearer by shaping changes into separate self-contained definitions. CL and EL client implementations are free to maintain versioning of data structures according to their preferences and may utilize optionality of JSON fields whenever it is suitable.
Capabilities
New engine_getCaps method is proposed (accepts no parameters, returns array of strings). The new method returns a list of Engine API methods (capabilities) that are currently supported by the corresponding EL client. Every method must be represented by all supported versions, i.e. engine_newPayloadV1 and engine_newPayloadV2 must be in the return list if EL currently supports both.
The method may not return itself in the list, a version suffix for this method doesn't seem relevant too.
It is assumed that CL clients request EL capabilities at the beginning of the Websocket session, and do this on demand while communicating via HTTP (periodically or after EL gets back from outage).
Note: Deprecated methods may be listed in the response to engine_getCaps if EL client does still support them.
Note: Optional methods are not required to be supported by EL clients, thus, may not be listed in the response to engine_getCaps.
Method status
The following method statuses are proposed:

Experimental -- yet under development, semantics and parameter set changes are expected. Such a method may have no version suffix (name of a feature as a suffix does seem more convenient, e.g.  engine_newPayloadWithdrwals, engine_getPayload4844 etc).
Final -- method specification is considered solid, changes may only be done in order to fix a bug.
Optional -- same as Final, but for optional methods.
Deprecated -- the method is deprecated and may no longer be supported by EL clients.

Note: These statuses are only used in the capabilities table described below, they don't explicitly appear in the client software implementations.
We propose deprecating a method as soon as it becomes irrelevant to the protocol as a general rule of thumb.
For instance, engine_exchangeTransitionConfigurationV1 has been used for the Merge transition and isn't required anymore, thus, should be deprecated. A post-Shanghai HF bumping some core methods to V3 would be a good moment to deprecate V1 versions of these methods introduced in Paris, or it even can be done some time after the Shanghai block event.
Table
Maintenance of the capabilities table is proposed to keep track of all Engine API methods and their statuses. A change of a status of any method version should be signified by the corresponding update in this table.
The table looks as follows:

Method
Source
Status

engine_newPayloadV1
Bellatrix
Final

engine_forkchoiceUpdatedV1
Bellatrix
Final

engine_getPayloadV1
Bellatrix
Final

engine_getPayloadBodiesByRangeV1
Deduplication
Optional

engine_getPayloadBodiesByHashV1
Deduplication
Optional

engine_newPayloadV2
Capella
Experimental

engine_forkchoiceUpdatedV2
Capella
Experimental

engine_getPayloadV2
Capella
Experimental

engine_getBlobsBundleV1
EIP-4844
Experimental

engine_exchangeTransitionConfigurationV1
Bellatrix
Deprecated

Note: "Source" column is added for clarity and may be omitted.
Note: We may want to keep a lean version of the table in the README which doesn't include old deprecated and stale experimental methods, and track statuses of entire set of methods and versions in a separate markdown file.
Directory structure
The following directory structure is proposed:
execution-apis/src/engine
├─ schemas/           # Schema files
│  ├─ paris.yaml
│  ├─ shanghai.yaml
│  ├─ deduplication.yaml
├─ common.md         # Common definitions and specification
├─ paris.md          # Paris HF
├─ shanghai.md       # Shanghai HF
├─ eip4844.md        # Specification of a feature
                     # that may be included in an upcoming HF

├─ deduplication.md  # Specification of a feature outside of any HF
├─ README.md         # Description of the process and lifecycle,
                     # summary of capabilities
├─ capabilities.md   # List of all capabilities

Method and spec document lifecycle
This section recommends a general procedure of working with spec documents and tracking Engine API methods/versions.
Every method and a new version of a method starts its way with Experimental status whether it is a new EIP that is planned for inclusion in one of the next HFs or simply a method providing an optimisation opportunity and tends to be deployed between HFs. This allows for a quick PR merge into Engine API specification and facilitates prototyping by adding experimental methods into the list returned by engine_getCaps call. Note that new methods should not be added to any document that is representing a HF that has already happened and which specification is considered as final.
Once the work on the HF gets more mature and the scope of changes to Engine API is more or less understood, it does make sense to move methods from multiple separate files into one hardfork.md file. When the work is done and HF specification is considered as final then statuses of involved methods should be changed to Final and the document should not have any further updates except for bug fixes.
For methods introduced outside of HFs the same process should be applied. When specification of optional method is finalized its status must be changed to the Optional instead of Final.
Whenever method specification is finalized its description should be added to the OpenRPC schema.
Deprecation of a method version should be reflected in the capabilities table by changing the status of the version to the Deprecated and removing it from the OpenRPC schema. Once this happens EL clients are free to get rid of the code of deprecated method version from their codebases, and remove it from the response to engine_getCaps method call. Note that documents containing specification of already deprecated methods must remain unchanged in the repository as other specification documents may refer to them.
Sometimes the deprecation flow may require more steps. For instance, let's see how deprecation of the engine_exchangeTransitionConfigurationV1 may work out:

CL clients remove hard dependency on making this call. It implies that if the method is still in the capabilities list, CL must follow the previous convention and call it periodically to prevent an EL client from complaining.
After that the status of this method is set to Deprecated and EL client devs remove its support.
CL client code may be cleaned up when all EL clients have removed engine_exchangeTransitionConfigurationV1 from the list of supported methods.

Thanks a lot for writing this up! :+1: from me on the capabilities method. I think this is important information for the CL to ascertain.
I do wonder if we really want/need optional methods? My worry is about the extra degree of freedom this gives clients and could lead to a bit of frustration if CLs rely on the method and certain ELs don't provide it.
I'm not sure I follow what the content of the HF markdown files will be vs. the general spec. It might be useful to consider the two paths to upgrading the functionality of a method:

version bump -- client knows it is getting the new functionality because the method name is different
hardfork -- client knows (to the best of its ability) it is getting the new functionality because the chain has been upgraded

I think we've all agreed we'd like to bump the version always, regardless of HF or not. In that case, it feels we may be over indexing a bit on the HF side of things? Would it be better to lay out the spec more in terms of functionality (see current layout of src/eth) and make notes about the functions behavior given certain HFs?
Somewhat related, should we consider whether API changes are totally backwards compatible? Contrived example: we add support for withdrawals and one day decide to remove them, remove it from the engine API, and then it isn't possible to run a chain that supports withdrawals?

@lightclient thanks a lot for your feedback!

I do wonder if we really want/need optional methods? My worry is about the extra degree of freedom this gives clients and could lead to a bit of frustration if CLs rely on the method and certain ELs don't provide it.

Optional methods is an alternative to fallback on error approach, demand for optionality is only represented by this proposal https://github.com/ethereum/execution-apis/pull/218. We may not care about optionality in the first iteration of setting up the process and debate on it later on in a separate iteration.

Would it be better to lay out the spec more in terms of functionality (see current layout of src/eth) and make notes about the functions behavior given certain HFs?

This is interesting idea, and it is more practical in some sense. Like we have a separate file for say newPayload which contains all versions of this function and call this file new-payload.md (I guess this is what you mean), it would be easier to spec them out this way and look for the history of changes. One of the downsides is that we have structures shared by different methods, e.g. ExecutionPayload shared by newPayload and getPayload but I think it's manageable. The other property of HF distinction is that we can discern yet experimental methods from solidified parts of the spec, but we may have experimental/ folder where files will be broken down not by functionality but rather by proposals that may affect all functionality at once, e.g. eip4844.md.

Somewhat related, should we consider whether API changes are totally backwards compatible?

I think we should follow the common sense, and make them backward compatible when it is reasonable, but do not make backward compatibility promise.

Contrived example: we add support for withdrawals and one day decide to remove them, remove it from the engine API, and then it isn't possible to run a chain that supports withdrawals?

This proposal suggests to have a reference table of all Engine API methods with their statuses, and instead of removing method from the spec we should mark it as Deprecated in that table and remain the spec for this method unchanged. So, one will be able to either dig out this method from client's code or just re-implement it.

Alternative structure with functional breakdown and statuses next to each version of a method (no need for a reference table):
├─ experimental/      # Experimental features
│  ├─ eip4844.md
│  │  ├─ Structures
│  │  │  ├─ BlobsBundleV1
│  │  │  ├─ PayloadAttributesV1
│  │  │  ├─ PayloadAttributesV2
│  │  ├─ Methods
│  │  │  ├─ getBlobsBundle
│  │  │  │  ├─ V1
│  ├─ feature-name.md
│  │  ├─ Structures
│  │  │  ├─ ExecutionPayloadFeatureName
│  │  ├─ Methods
│  │  │  ├─ newPayloadFeatureName
│  │  │  ├─ getPayloadFeatureName
│
├─ schemas/           # Schema files
│  ├─ blob.yaml
│  ├─ capability.yaml
│  ├─ configuration.yaml
│  ├─ forkchoice.yaml
│  ├─ payload.yaml
│  ├─ payload-body.yaml
│
├─ common.md
│  ├─ Underlying protocol
│  ├─ Versioning
│  ├─ Message ordering
│  ├─ Load-balancing and advanced configurations
│  ├─ Errors
│  ├─ Timeouts
│  ├─ Capabilities
│  │  ├─ getCaps
│
├─ configuration.md
│  ├─ Structures
│  │  ├─ TransitionConfigurationV1
│  ├─ Methods
│  │  ├─ exchangeTransitionConfigurationV1
│  │  │  ├─ V1: Deprecated
│
├─ forkchoice.md
│  ├─ Structures
│  │  ├─ ForkchoiceStateV1
│  │  ├─ PayloadAttributesV1
│  │  ├─ PayloadAttributesV2
│  ├─ Routines
│  │  ├─ Payload building
│  ├─ Methods
│  │  ├─ forkchoiceUpdated
│  │  │  ├─ V1: Final
│  │  │  ├─ V2: Draft
│
├─ payload.md
│  ├─ Structures
│  │  ├─ ExecutionPayloadV1
│  │  ├─ ExecutionPayloadV2
│  │  ├─ PayloadStatusV1
│  │  ├─ WithdrawalV1
│  ├─ Routines
│  │  ├─ Payload validation
│  │  ├─ Sync
│  │  ├─ Payload building
│  ├─ Methods
│  │  ├─ newPayload
│  │  │  ├─ V1: Final
│  │  │  ├─ V2: Draft
│  │  ├─ getPayload
│  │  │  ├─ V1: Final
│  │  │  ├─ V2: Draft
│
├─ payload-body.md
│  ├─ Structures
│  │  ├─ ExecutionPayloadBodyV1
│  ├─ Methods
│  │  ├─ getPayloadBodiesByHash
│  │  │  ├─ V1: Final
│  │  ├─ getPayloadBodiesByRange
│  │  │  ├─ V1: Optional

cc @lightclient

New engine_getCaps method is proposed

While it's not wrong to have a capabilities function, reality is such that users upgrade their systems independently of each other and/or run round-robins on multiple versions of the same API - hence, when consuming the API we rarely assume that such support stays stable.
Instead, in the ideal case we would standardise:

an error code for when an API is missing (to cover "old clients" not yet supporting an API") (ie 404)
an error code for when an API has been removed permanently (ie 410)

Clients then operate the following way:

the try the newest API - if this gives a 404, they fall back on an earlier version, while at the same time notifying the user that they likely need to upgrade the API server
if the earlier version gives 410, they can notify their users they need to upgrade their API client

This optmistic strategy works well in practice: during testnets and development, it uses the latest version available without making any assumptions about support in the EL - then, when the feature is "released", it takes advantage of the new version where possible (without any messy assumptions about fork activation epochs etc) and gracefully falls back - finally, once the deprecation period for the old version has passed, old consumers can degrade gracefully as well (by showing an appropriate error).
This strategy also implies that when we add new API:

the new API should  ideally cover the "previous" version (ie V-1 but not necessarily V-2)
ditto implementations should provide both API for a few versions

It is unlikely we would use a getCapabilities function with the above in place because that only introduces an extra call / complexity that still has to be handled at the per-call level - in other words: capabilities introduce an additional way interacting with the EL can fail but not much benefit, because the same information also has to be dealt with on a per-call basis.

run round-robins on multiple versions of the same API

I am not sure I understand this in the context of Engine API. If CL drives multiple ELs (not sure that this is what you meant by running round-robins) then there should be a multiplexer propagating calls to all of them, if Engine API versions of those ELs are different it can be an issue with whatever approach we take.

an error code for when an API is missing (to cover "old clients" not yet supporting an API") (ie 404)

We already have such error: -32601: Method not found.

if the earlier version gives 410, they can notify their users they need to upgrade their API client

Considering we agree that V-1 stays for quite long period of time, the case when 410 would be hit is pretty rare. I can imagine this happening when we're deprecating exchangeTransitionConfiguration as this method is going to be removed entirely. Based on 404 only CL may surface a complain to a user that it doesn't see a required Engine API method and either CL or EL should be upgraded. Supporting such status implies EL remembers all deprecated methods which doesn't seem to be worth it.

This optmistic strategy works well in practice

CL client will fallback to V-1 every method call until EL is upgraded, this is suboptimal. There can be more fancy logic when CL tries V+1 every Nth calls and if succeeded starts using the new version onwards. While getCapabilities allows for the following strategy:

CL knows the latest version of each method it is compatible with
CL calls getCapabilities every N minutes in parallel to the main communication flow, and if newer version of existing method or a new method is found, CL starts using it from now and onwards. This is why backwards compatibility at least up to V-1 is important.
This strategy may also utilise 404 to stop calling deprecated method if it is expected to disappear

IMO, both strategies may co-exist, if no CL team will use getCapabilities then we can avoid specifying and implementing it. Standardising 410 doesn't sound like a good idea to me because it requires a list of old methods kept by EL client software

CL client will fallback to V-1 every method call until EL is upgraded, this is suboptimal.

It's suboptimal but also temporary and cheap - further, it's those that  haven't upgraded that pay the cost, which is appropriate: once everyone has upgraded (as happens eventually), the cost goes back to 0.

There can be more fancy logic

This fancy logic is surface area for bugs and breaks redundant round-robin setups - it's easier to not make this assumption thanks to how cheap it is to make a call, and if that fails, make another (ie this is really simple and cheap: you maintain a connection then try one after the other) - it's also foolproof in that the consumer doesn't have to maintain state beyond the "logical" request. Capabilities calls make simple scenarios more simple and optimal, but make more advanced scenarios harder or impossible.

IMO, both strategies may co-exist

True, it's not like it disturbs anyone, except those that have to implement the server and keep it up to date ;)

Standardising 410 doesn't sound like a good idea to me because it requires a list of old methods kept by EL client software

it's not required - it's more of a quality of implementation matter where during the transition period, we often get users that haven't upgraded part of their setup - 410 (or a reasonable -xxxxx code, whichever fits the current design better) give a way to give them a better error message - servers don't have to support this, but it can be used to good effect.

True, it's not like it disturbs anyone, except those that have to implement the server and keep it up to date ;)

Once getCapabilities is implemented there is no need to update the list, the list should be self-adjusting. I feel like implementation cost of this method is really low

@arnetheduck

it's those that haven't upgraded that pay the cost

This is not always true. If https://github.com/ethereum/execution-apis/pull/218 makes into the spec, CL clients will have to use fallback approach or alternatively cache Engine API server configuration.

I'm 👍 on the functional structure of the spec. I'll let CL teams weigh in on the desirability of getCaps.

A couple of other Engine API things to raise if there is a time:

https://github.com/ethereum/execution-apis/pull/314
https://github.com/ethereum/execution-apis/pull/218

Will just toss in that I like fork-diff format. It works well for CL specs. This path, imo, is optimized for writers and sophisticated readers (e.g. client teams). "What do I need to know/change in relation to what I already know".
Given this is not a user facing API, I bias toward optimizing for hte sophisticated user at the potential cost of slowing of onboarding of new users of the api

I bias toward optimizing for hte sophisticated user at the potential cost of slowing of onboarding of new users of the api

OpenRPC schema is supposed to contain a reference of all stable and not yet deprecated Engine API methods, i.e. methods that EL clients must support. Therefore, it should alleviate the pain of non-experienced reader and reduce the entrance barrier.

Strongly support engine_getCapabilities (provided it's called engine_getCapabilities and not engine_getCaps since none of the other method names are truncated).
In fact I've already stubbed out this functionality in lighthouse based on earlier talks that a function of this nature would be supported. Lighthouse essentially caches a data structure that enumerates the methods that the execution engine supports. It's trivial to implement functionality that would refresh this data structure (with a call to engine_getCapabilities) periodically or if lighthouse has detected the execution engine has gone offline (to update to a newer version for example).

reality is such that users upgrade their systems independently of each other and/or run round-robins on multiple versions of the same API - hence, when consuming the API we rarely assume that such support stays stable.

lighthouse docs explicitly state that we do not support 1:many or many:1 configurations between CL and EE. You would certainly require a multiplexer to support this. And if you've already written a custom multiplexer then it should be trivial to handle parsing the response from engine_getCapabilities and returning the highest version method supported by all execution engines.

Resolved by the following PRs:

https://github.com/ethereum/execution-apis/pull/327
https://github.com/ethereum/execution-apis/pull/364
https://github.com/ethereum/execution-apis/pull/379


Cleanup: Initialize instance variable before use to avoid Ruby warnings

Description
This avoids a Ruby warning emitted on use.
By initializing the variable before use, this is an easy win against the sea of warnings output.
Testing

Run the test with RUBYOPT=-W2
See that @decorated_collection is not among the output

Hi there! Any news?

Ping @codebycliff

Ping @codebycliff

@olleolleolle can you please rebase your branch on master?

@olleolleolle can you please rebase your branch on master?

I could make an attempt, in about 12 hours.

I could make an attempt, in about 12 hours.

Haha, the issue is created by: me defining a variable, so the test must be altered to match that new situation!

Haha, the issue is created by: me defining a variable, so the test must be altered to match that new situation!

@olleolleolle thank you! Ping @codebycliff

@olleolleolle thank you! Ping @codebycliff

ping @codebycliff

The warning was removed from Ruby years ago.


🔄 Synced file(s) with jhudsl/OTTR_Template

Synced local file(s) with jhudsl/OTTR_Template.

Changed files

Synced local directory .github/workflows/ with remote directory .github/workflows/

This PR was created automatically by the repo-file-sync-action workflow run #9390587381

Yay! This is working @caalo See https://github.com/fhdsl/Intro_to_R/actions/runs/9400814547
Feel free to merge at your leisure.
I'm bringing what we learned here back to the main OTTR Template as well: https://github.com/jhudsl/OTTR_Template/pull/761


(WIP) Context Threading

Relevant issue: #24
This is WIP but brings us most of the way there. The right inventions are showing up but some other bad ones are too.
cargo run --release --bin=compress data/basic/map.json -i1 -a1 gets us the invention we love!
inv0 (2.46x wrt orig): utility: 252 | final_cost: 1111 | 2.46x | uses: 2 | body: [inv0 arity=1: (Y (lam (lam (if (is_nil $0) nil (cons (#0 $0) ($1 (cdr $0)))))))]
which rewrites things to
(inv0 (lam (+ (car $0) (car $0))))
(inv0 (lam (- (car $0) 1)))
which is great!
However when we increase the arity there are some other inventions whose utility is mistakenly marked higher.
cargo run --release --bin=compress data/basic/map.json -i1 --inv-candidates=11
Note that when running this inv10 is the invention we know and love, and it even has the best final_cost (ie cost after rewriting)! However it's ranked so low because all the other inventions have mistakenly high utility.
The high utility seems to come from 2 things:

first the inventions seem to think they're gaining some sort of multiuse (?) benefit from using #0 $i to abstract more variables.
second the inventions seem to be able to use the single ivar #0 to do ctx-threading with multiple $i that definitely dont refer to the same variable. It makes some sense to me that the arg that that point would be (lam $0) for all of them so I can see why multiuse would merge them, but also this seems wrong.

oh wait, actually if we assume the arg is lam 0 in all cases then this is totally valid! The only problem is my first point that they seems to gain some multiuse benefit from doing #0 $i with an argument of lam $0.

Update: turns out --no-other-util got rid of the mis-ranking so must be structure penalty. This checks out with why util number differences werent aligned with the 100s too!
todos:

[x] fix the structural penalty issue above
[ ] write a set of super simple context threading tests for future testing / debugging
[ ] write a test for threading multiple variables since I don't think I've ever done that
[ ] extend the map example so that it actually finds map, as in it brings out car $0 as the thing thats threaded instead of just $0 getting threaded. Oh wait or no is that not what context threading is bc thats more what full double beta inversion is and requires the inner search?
[ ] make sure context threading is a fully optional feature
[ ] check the timing effects of context threading
[ ] try DC list domain

Somewhat amazingly, the mis-ranking actually went away (when running the exact same command) after all the changes to main! No more cost mismatches here. I'm not exactly sure where it went, but if it ever does become a future issue we'll see it coming thanks to cost_mismatch

Ah actually the issue is still here but it has way less effect. In fact now it decreases the utility (by around 200 in one case) instead of increasing it, and this is with no_other_util.
With other_util included there's a second instance of this, though they look very similar so I suspect theyre the same issue and one just happened to be revealed by the slight difference in pruning that comes from a diff utility function.
For simplicity to track just one of these at a time lets look at
cargo run --release --bin=compress data/basic/map.json -i1 --inv-candidates=10000000000000000 --no-other-util --show-rewritten
in particular candidate 16
*** expected cost 1517 != final cost 1313
16: [cost mismatch] utility: 1212 | final_cost: 1313 | 2.08x | uses: 2 | body: [fn_0 arity=2: (Y (lam (lam (if (#0 $0) nil (cons (#1 $0) ($1 (cdr $0)))))))]
rewritten: (programs (fn_0 is_nil (lam (+ (car $0) (car $0)))) (fn_0 is_nil (lam (- (car $0) 1))))

try DC list domain
cargo run --release --bin=compress data/dc/dc_logo_log.json --dc-fmt
this still performs poorly, submitting as a separate issue #73

Looking good, nuts-bolts and logo give the right results.
Todo still:

[ ] write a test for threading multiple variables since I don't think I've ever done that even though it should be supported by our current context threading
[ ] cargo bench though from brief checks it seems fine

cargo bench against main: honestly performance seems about the same it even improved in most benchmarks. I think our benchmarks are just too easy and all suuuuch low time


Upgrade ruby/setup-ruby v1.161.0 -> v1.165.1

This PR contains the following updates:

Package
Type
Update
Change

ruby/setup-ruby
action
minor
v1.161.0 -> v1.165.1

Release Notes

ruby/setup-ruby (ruby/setup-ruby)
v1.165.1: Fix rubygems: latest to work with Rubies < 2.3
Compare Source

https://github.com/ruby/setup-ruby/pull/557

v1.165.0: Fix 'rubygems: latest' for Ruby < 3.0
Compare Source

https://github.com/ruby/setup-ruby/pull/551

v1.164.0: Add ruby-3.3.0 on Windows
Compare Source
v1.163.0: Add ruby-3.3.0
Compare Source
v1.162.0: Add ruby-3.3.0-rc1
Compare Source

[ ] If you want to rebase/retry this PR, check this box

The Java Platform team wishes you a happy festive period. Here is the suggested commit message:
Upgrade ruby/setup-ruby v1.161.0 -> v1.165.1

Renovate be like

Header generated and modified from Sweaterify.

The Java Platform team wishes you a happy festive period. Here is the suggested commit message:
Upgrade ruby/setup-ruby v1.161.0 -> v1.165.1

Renovate be like

Header generated and modified from Sweaterify.


Implementation of insertion sort in c, in task 3

@Goyalscode Please delete the .exe and .o files

I have deleted the .o and .exe files.
Please review it again @chandrikadeb7


Update showcase.js

cc @ide & @marcshilling

@facebook-github-bot shipit

:/  sorry

no worries, I wish all master breaks were like this :)

Oops, my bad!


error while install ppfuz on macbook pro m1

Describe the bug
A clear and concise description of what the bug is.
visit this https://github.com/dwisiswant0/ppfuzz/issues/28

Not a reconftw issue


next-typescript uses ts-loader again

Babel 7 supports TypeScript transpilation, Next.js 7 and latest next-typescript use Babel 7, but the limitation creates breaking changes.
On large project, developers create index.js (or index.ts ) to encapsulate the API calls from the web app into a single folder like the following.
// api.ts
export { ClassA } from "package-1";
import ClassB from "package-2";
import ClassC from "package-3";

export { ClassB, ClassC };

// main.ts
import { ClassA, ClassC } from "./api";

Babel 7 does not support these re-export types and throw some warnings. (For more detail: https://github.com/babel/babel/issues/8361#issuecomment-428756919)
Currently, Next.js 7 block HMR when some warnings are shown (For more detail: https://github.com/zeit/next.js/issues/5429). This means TypeScript developers creating index.ts cannot use HMR. Even if the HMR blocking issue is solved, Babel 7 throws re-export warnings.
So next-typescript should use ts-loader instead of Babel ( @babel/preset-typescript ) because ts-loader does not throw warnings and block HMR.

Yeah this is a pretty big limitation with the babel transpilation which I also hit, and they can't fix it. Would be nice to be able to choose whether to use ts-loader or babel.

Would export { default as ClassB } from 'package-2'; work for you?

@sorenhoyer It does not work...

To be possible to re-export some interfaces using next-typescript, should use ts-loader again, but I think it is difficult.
So I just created a package to use TypeScript using ts-loader with Next.js.
https://github.com/jagaapple/next-tsc
Thank you.

@jagaapple – cool workaround, but unfortunately that makes compilation take so long that the HMR client times out waiting for the hot update. Sigh.

Next.js will have typescript support by default soon.