Laravel 10: DB::transactionLevel() shows incorrect level, if DDL operation was done

Laravel Version
10.28.0
PHP Version
8.2.11
Database Driver & Version
10.10.2-MariaDB-1:10.10.2+maria~ubu220
Description
We encounter that DB::transactionLevel() shows an incorrect level after a DDL operation was done, which does commit implicitly. Consider this code snippet which is the current behaviour:
Log::debug(DB::transactionLevel()); // 0
DB::beginTransaction();
Log::debug(DB::transactionLevel()); // 1
DB::commit();
Log::debug(DB::transactionLevel()); // 0

DB::beginTransaction();
Log::debug(DB::transactionLevel()); // 1
// some DDL operation which closes the transaction
Log::debug(DB::transactionLevel()); // 1

This is problematic on different levels. First, getting the correct transaction level is important to be able to close all pending transactions. Second, due to a current change (see: https://github.com/laravel/framework/issues/35380), DB::commit() and DB::rollback() both throw an exception if no active transaction is present. That means after beginTransaction and a DDL statement, this code:
if (DB::transactionLevel() > 0) {
  DB::commit();
}

will fail.
Steps To Reproduce
DB::beginTransaction();
Schema::dropIfExists('test');
Schema::create('test', function (Blueprint $table) {
    $table->id();
});
if (DB::transactionLevel() > 0) {
  DB::commit();
}

I just confirmed this behaviour in a minimal example, happy to provide a repo if needed.

Hi there,

We encounter that DB::transactionLevel() shows an incorrect level after a DDL operation was done, which does commit implicitly

I don't believe commit should be done explicitly but you should be able to override db.transactions and handle it based on the requirements.
https://github.com/laravel/framework/blob/a47df681bf4b72cb11e38f7ee7c285406ca934e0/src/Illuminate/Database/DatabaseServiceProvider.php#L77-L79
The current db.transactions is well-tested and working as intended.

Also, this is documented here: https://laravel.com/docs/10.x/database#implicit-commits-in-transactions

@crynobone Thank you very much for the documentation link, but I am unsure if there is a misunderstanding here. That documentation explicitly only applies to the statement and unprepared methods of the DB facade:

When using the DB facade's statement and unprepared methods within transactions you must be careful to avoid statements that cause implicit commits.

But the problem here occurs even if these methods are not used, but if Laravel methods like Schema::XXX are used. Please, I invite you to have again a look at the Step to Reproduce. I can understand that it is difficult (but probably not impossible) to track transaction level for raw SQL statements done by the user. However, in the example above we only use Laravel database abstraction methods, so the framework should very well be able to track the transaction level correctly internally there, and frankly this is what I would expect.

I don't believe commit should be done explicitly

I am not sure if I understand you correctly here. If we start a transaction manually via DB::beginTransaction() we have to explicitly commit at the end. This consumption of the Laravel API is officially documented here: https://laravel.com/docs/10.x/database#manually-using-transactions
I assume you refer to the other case, where transactions are run inside the callback of DB::transaction? I just reproduced, however that also in this case the internal transaction level is not updated correctly on a DDL operation done using Schema::XXXmethods. This example:
        DB::transaction(function () {
            Schema::dropIfExists('test');
            Schema::create('test', function (Blueprint $table) {
                $table->id();
            });
            $this->printTransactionLevel();
        });

Also fails with an "There is no active transaction" PDOException:
[2023-10-19 20:39:11] local.ERROR: There is no active transaction {"view":{"view":"/var/www/laravel_docker/resources/views/index.blade.php","data":{"errors":"
Illuminate\\Support\\ViewErrorBag {#277<samp data-depth=1 class=sf-dump-expanded>
  #bags: []
</samp>}

"}},"exception":"[object] (Spatie\\LaravelIgnition\\Exceptions\\ViewException(code: 0): There is no active transaction at /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:194)
[stacktrace]
#0 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php(194): PDO->commit()
#1 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(469): Illuminate\\Database\\Connection->commit()
#2 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(353): Illuminate\\Database\\DatabaseManager->__call('commit', Array)
#3 /var/www/laravel_docker/app/Http/Controllers/IndexController.php(27): Illuminate\\Support\\Facades\\Facade::__callStatic('commit', Array)
#4 /var/www/laravel_docker/resources/views/index.blade.php(3): App\\Http\\Controllers\\IndexController->run()

This is the same exception which occurs in the example code provided in the Steps to Reprocude section.
The transaction level at the end of the callback also is 1, instead of 0. This is an indication that the Schema::dropIfExists and Schema::create method do not correctly update the Laravel internal transaction level variable.

If you refer to link from documentation (MySQL documentation) implicit
commit include command used in Schema
https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html
On Fri, 20 Oct 2023, 4:44 pm mrckzgl, @.***> wrote:

@crynobone https://github.com/crynobone Thank you very much for the
documentation link, but I am unsure if there is a misunderstanding here.
That documentation explicitly only applies to the statement and unprepared
methods of the DB facade:
When using the DB facade's statement and unprepared methods within
transactions you must be careful to avoid statements that cause implicit
commits https://dev.mysql.com/doc/refman/8.0/en/implicit-commit.html.
But the problem here occurs even if these methods are not used, but if
Laravel methods like Schema::XXX are used. Please, I invite you to have
again a look at the Step to Reproduce. I can understand that it is
difficult (but probably not impossible) to track transaction level for raw
SQL statements done by the user. However, in the example above we only use
Laravel database abstraction methods, so the framework should very well be
able to track the transaction level correctly internally there, and frankly
this is what I would expect.
I don't believe commit should be done explicitly
I am not sure if I understand you correctly here. If we start a
transaction manually via DB::beginTransaction() we have to explicitly
commit at the end. This consumption of the Laravel API is officially
documented here:
https://laravel.com/docs/10.x/database#manually-using-transactions
I assume you refer to the other case, where transactions are run inside
the callback of DB::transaction? I just reproduced, however that also in
this case the internal transaction level is not updated correctly on a DDL
operation done using Schema::XXXmethods. This example:
    DB::transaction(function () {
        Schema::dropIfExists('test');
        Schema::create('test', function (Blueprint $table) {
            $table->id();
        });
        $this->printTransactionLevel();
    });

Also fails with an "There is no active transaction" PDOException:
[2023-10-19 20:39:11] local.ERROR: There is no active transaction {"view":{"view":"/var/www/laravel_docker/resources/views/index.blade.php","data":{"errors":"
Illuminate\Support\ViewErrorBag {#277
#bags: []
}

"}},"exception":"[object] (Spatie\LaravelIgnition\Exceptions\ViewException(code: 0): There is no active transaction at /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php:194)
[stacktrace]
#0 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/Concerns/ManagesTransactions.php(194): PDO->commit()
#1 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Database/DatabaseManager.php(469): Illuminate\Database\Connection->commit()
#2 /var/www/laravel_docker/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php(353): Illuminate\Database\DatabaseManager->__call('commit', Array)
#3 /var/www/laravel_docker/app/Http/Controllers/IndexController.php(27): Illuminate\Support\Facades\Facade::__callStatic('commit', Array)
#4 /var/www/laravel_docker/resources/views/index.blade.php(3): App\Http\Controllers\IndexController->run()
This is the same exception which occurs in the example code provided in
the Steps to Reprocude section.
The transaction level at the end of the callback also is 1, instead of 0.
This is an indication that the Schema::dropIfExists and Schema::create
method do not correctly update the Laravel internal transaction level
variable.
—
Reply to this email directly, view it on GitHub
https://github.com/laravel/framework/issues/48777#issuecomment-1772328265,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABKHJSXSO3BL7SPGT27EFTYAI2YDAVCNFSM6AAAAAA6HZ6K3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZSGMZDQMRWGU
.
You are receiving this because you were mentioned.Message ID:
@.***>

@crynobone Yes, but that documentation is from dev.mysql.com. It does not apply to Laravel. As stated, the documentation for Laravel API you referred to only applies to DB::statement and DB::unprepared. It does not apply to other Laravel methods. I invite you to please completely and carefully read my last comment with all the points I presented. A response after just one minute from yours via email indicates that this might not have been done.

If you have any suggestion to override implicit commit imposed by MySQL and PDO feel free to send in a PR.

Frankly, this is not my task here. I am here to make you, the maintainers of Laravel, aware of a bug in your code, or well if you want to state that as limitation you do not want to address, as a problem in your documentation. I would assume, that you as maintainers have an interest in quality code, and / or quality documentation, but I might be wrong. If this bug turns out to be a limitation you do not want to address, I would kindly ask you to update your documentation so that it correctly reflects the problem I posted initially. Trying to argue that the documentation applies here, if it explicitly does not, is not very helpful or friendly to be honest.
So, to make it more direct: If you do not want to implement DB:transactionLevel() such that it tracks all transaction related state changes Laravel does, I would kindly ask you to include this in the documentation you already pointed to here: https://laravel.com/docs/10.x/database#implicit-commits-in-transactions and also in the actual API reference here: https://laravel.com/api/10.x/Illuminate/Database/ConnectionInterface.html#method_transactionLevel and here: https://laravel.com/api/10.x/Illuminate/Database/Concerns/ManagesTransactions.html#method_transactionLevel
thank you very much

@crynobone Thank you for reopening again. Just another idea to handle DDL statements: Why no open a transaction inside Schema::XXX after any possible implicit commit and of course only if a transaction was active beforehand. This way, transactionLevel stays the same. Of course, this behaviour then should ideally be documented for the respective Schema:XXX methods.
best

I wrote a comment on antoher issue, about having the posibility to resolve a separate db connection when the user wants to issue implict commit statements. Not sure if this is possible/feasible though.
https://github.com/laravel/framework/issues/35380#issuecomment-1720995440
DB::beginTransaction();

DB:: isolatedConnection('mysql')->statement('CREATE TABLE ...')

DB::commit();

Yes, but that documentation is from dev.mysql.com. It does not apply to Laravel. As stated, the documentation for Laravel API you referred to only applies to DB::statement and DB::unprepared.

The Laravel docs share a link to the MySQL resource listing all database command making an implicit commit. So yes, this is clearly documented and everything works as expected and there is no way Laravel can do anything different. It is how MySQL behaves.
The only possible change would be listing migrations as another reason for imilicit commits additionally to the DB::unprepared and DB::statement method.

The Laravel docs share a link to the MySQL resource listing all database command making an implicit commit. So yes, this is clearly documented and everything works as expected and there is no way Laravel can do anything different. It is how MySQL behaves.

I am a quite surprised to have to conduct such discussion. What you write is a wrong statement. Let's again cite the docs:

When using the DB facade's statement and unprepared methods within transactions you must be careful to avoid statements that cause implicit commits. These statements will cause the database engine to indirectly commit the entire transaction, leaving Laravel unaware of the database's transaction level.

Let's rephrase this: If users use DB::statement or DB::unprepared they have to be careful to avoid DDL statements. The docs do not state, that users need to be careful to avoid DDL statements in other Laravel methods, such as Schema::create and the like. In other words: That users need to be aware of other Laravel methods which might cause implicit commits is not stated in the paragraph above. That a link to the MySQL resource is given which lists all possible SQL statements (which can be explicitly used as argument inside DB::unprepared and DB::statement) does not mean that users need to be aware of DDL SQL statements, which Laravel might use itself internally in other Laravel methods. That connection is pretty far fetched and is not mentioned at all in the docs. It is also far fetched, because the Laravel framework has all the information needed to know if their Schema:XXX methods will issue an implicit commit.

there is no way Laravel can do anything different.

From my point of view, here are some possible things Laravel could do different. Let's take Schema::create as example.
If a database backend is used which does an implicit commit after create table and if Schema::create actually issues create table Laravel could:

Decrease DB::transactionLevel accordingly (such that this method correctly reflects the transaction level), or
as suggested above: Start a new transaction if there was any beforehand. That way the code inside DB::transaction will be more consistent as at any point in user space exactly (more precise: at least) one transaction will be active.

A more generic approach might be to query the database driver if the transaction level has changed at beginning and end of Schema::create and act accordingly. This approach might also catch all transaction changes in DB::statement and DB::unprepared.

The only possible change would be listing migrations as another reason for imilicit commits additionally to the DB::unprepared and DB::statement method.

If for some reason a change in Laravel behaviour is not feasible or too expensive to implement at the moment: Of course, exactly! That is what I was referring to in https://github.com/laravel/framework/issues/48777#issuecomment-1772354828. More precise, not only migrations need to be listed in the docs, but all methods for which Laravel might cause an implicit commit internally. I am explicitly mentioning this, as for example Schema:create can be used outside of a migration, and some frameworks actually do this in production code.
best

Closing this for now, but anyone is still free to submit a PR to https://github.com/laravel/docs if you really want to pursue it.


Extend pash with pure command

This allow users to wrap pure commands of their scripts without having to write annotations for them. It would make especially a lot of sense for functions.

It could be used as follows:
pure -i in1 in2 in3 -o out1 out2 out3 -- comm args

where -i indicates the input files, -o indicates the output files, and everything after -- is the command to run.
This should be implemented as the identity (for when PaSh decides not to compile a fragment that contains it) and when PaSh sees it, it should know the inputs and outputs of that command.

Subsumed by #453


Show splash screen above Cage

Hi everyone, much love from Italy!
I am currently building an embedded system for a car infotainment, and decided to opt for Cage, since it seemed the most sensible choice.
I tried (with no success, and not much experience either) to keep the Plymouth (or any other splash screen) active after starting Cage, but to me it seems not possible. Not sure if it is even supported by hardware/software implementations.
Let me know if there could be any workaround or solution to this, since i would really need to keep up that splash screen to hide some of the loading that is happening inside of the application. (And i think there might be quite a lot of use cases for this, mostly for embedded systems).
Thanks,
Man

I don't know if it is Cage or elsewhere, but I think the "framebuffer" (DRM devce) is cleared at Cage initialization. Are you not able to show a kind of splash screen directly in the started application?

I think that'd be a very nice feature.
 We'd need to avoid doing that modeset for outputs which are already turned on and if we can page-flip with our own swapchain, then delay the render loop until the client commits its first buffer.

I don't know if it is Cage or elsewhere, but I think the "framebuffer" (DRM devce) is cleared at Cage initialization. Are you not able to show a kind of splash screen directly in the started application?

Hi Jonathan!
Yes well, i implemented that splash screen on the application as well since i could not find a solution using Cage, but it is sub-optimal. The Plymouth splash disappears, then there is a black screen, followed by a white screen briefly, and that is not really something I would want on my finished product (bear in mind that right now its only a personal project, so it doesnt really matter that much), but i would prefer something more polished and OEM-like.

I think that'd be a very nice feature.
 We'd need to avoid doing that modeset for outputs which are already turned on and if we can page-flip with our own swapchain, then delay the render loop until the client commits its first buffer.

Exactly Simon! It would be very cool, and not limited to this case, but as i said it could be used for other purposes, like terminals, vending machines etc.
I am not sure about what you wrote tho, i didn't really understand much of it ahahah. But correct me if i'm wrong, in simple terms what you are saying is that we should try to "disable" the video output until the first frame of the app renders, right?
My goal as i said is to have a clean startup.
Power Button > Splash Screen (with no flickers, black or white screens) > Loaded App (which in this case is React Carplay by Rhys Morgan
I modified it a bit but still is not as nice as i would like the final project to look like...

Does the client immediately commit its buffer @The-Man11 ? Are you trying to display splash until the client commits, or are you trying to hide the client's loading state. You mentioned "then there is a black screen, followed by a white screen briefly,". If cage waited for the client's frame buffer commit and the client committed with a white screen, it would be displayed regardless. You also asked for a workaround. This is what I though of. Use Plymouth's Fade-In option. When cage loads spawn your app and immediately the splash screen again.  Post demonstrative videos if you try any workarounds, I'm curious of how it'd look like.


Create child upon directory creation

Signed-off-by: Pradeep Kumar K J<EMAIL_ADDRESS>Proposed changes
When a directory for the child device supported operation is created in /etc/tedge/operations/c8y then the mapper should send the 114 smartest message to the c8y cloud to create a new child device.
Types of changes

[x] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[x] Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
[ ] Documentation Update (if none of the other choices apply)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

Checklist

[x] I have read the CONTRIBUTING doc
[x] I have signed the CLA (in all commits with git commit -s)
[x] I ran cargo fmt as mentioned in CODING_GUIDELINES
[x] I used cargo clippy as mentioned in CODING_GUIDELINES
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation (if appropriate)

Further comments

Addressed with #1538


Fix & Simplify .vox signature check

and make version check be only 150 not over 150
because everytime a version changes, the way to load the file gets different, we would first have to add support, then allow
the old version check was big and didnt work(it just checked for the first letter, and if the first letter was not "V" and "O" and "x" AND " " THEN it would fail, not good, so i made this!

@CrezyDud Nice! Thanks for the review!

Thanks for merging!


Define 'short'

Currently Rustfmt treats certain items differently if they are 'short'. For example, it will but a short struct literal on one line:
// Short
let x = Foo { x: 42 };

// Not short
let opts = Options {
    flag_package: vec![],
    flag_jobs: None,
    flag_features: vec![],
    flag_all_features: false,
    flag_no_default_features: false,
};

As @joshtriplett points out in https://github.com/rust-lang-nursery/fmt-rfcs/issues/31#issuecomment-268343997, we should have one global definition of 'short' rather than a different definition every time.
I see two ways to define 'short' - absolute characters (e.g., 25) or proportion of max line length (e.g., 1/4). One might argue that no matter how long the line length, what makes (e.g.) a struct literal readable on one line does not change. On the other hand, people who prefer longer lines might also prefer more on one line in these cases.

Places where I think this should kick in (off the top of my head) are struct variants (discussed in #31), struct literals, and function calls. There might be other places we should consider this but currently don't. Rustfmt currently uses different values for all of these:
fn_call_width: usize, 60,
struct_lit_width: usize, 16,
struct_variant_width: usize, 35,

(from https://github.com/rust-lang-nursery/rustfmt/blob/master/src/config.rs).
That actually suggests we shouldn't have a universal value. I'd be happy for both struct values to be 25-ish, but function calls should probably be longer (we might reconsider the value, or if we need one at all given that we will almost certainly change how function calls are otherwise formatted).

When formatting manually I use "short" limit == ideal_width == max_width.
I remember I was surprised when I saw rustfmt splitting lines that obviously fitted into 100 characters.
Where is "short" usually used?
I'd understand if it were used in, e.g. enum definitions to split long variants, but not in function bodies.

@nrc I honestly find it surprising that rustfmt has a different length for wrapping function calls, rather than using the normal line length.  Its long length compared to the other two seems like an artifact of that; I'd advocate for using the line length for function calls.
For the other two, I don't think it'd hurt to say "a third of line length" or "half of line length" for both, if we decide to define "short" here.
That said, I think I'd still prefer to just use the normal line length here, and only limit struct literals and struct variants by the number of fields.  I'd be fine with "at most three fields before splitting onto multiple lines, even if shorter than the line length", for instance.  And that rule seems much easier to manually apply than counting characters.
I'd really like to avoid the scenario where I can't trivially recreate rustfmt's style by following easy-to-manually-apply rules with a bit of editor assistance.  That's one of the biggest things that bothers me about a character-count-based definition of "short".

The use case for the function args is something like this:
    foo(an_object.call1().call2().field, something.foo + 42, top_fn(nested_fn(deep), bar(), baz()));

    // vs.

    foo(an_object.call1().call2().field,
        something.foo + 42,
        top_fn(nested_fn(deep), bar(), baz()));

That line (with a single 4 space indent) fits in 100 chars, but is pretty horrible to read, I much prefer to split the args to multiple lines and would do so myself if formatting manually.

With these heuristics we get in to formatting mostly for aesthetics/subjective readability. I realise that we lose some of the appeal of a very tight spec here, but I believe there is scope for making Rustfmt a better tool by using these somewhat arbitrary heuristics. My experience has been that we closely match what people do manually by doing this.

That said, I think I'd still prefer to just use the normal line length here, and only limit struct literals and struct variants by the number of fields.

This was not enough in the past - the size of fields (and types/arguments) varies widely and people when manually formatting tend to take into account actual size as well as number of fields.

I'd really like to avoid the scenario where I can't trivially recreate rustfmt's style by following easy-to-manually-apply rules with a bit of editor assistance.

I think this is subtly but importantly wrong. I think that it is important that we have a spec that can be easily applied by both tools and real people. I think that is should be an explicit non-goal that tools and people would apply exactly the same formatting. Basically, people and machines work too differently for this to end well. Furthermore, I don't see any advantage in people and tools coming up with exactly the same formatting (obvs there are advantages to the spec being manually implementable and for a tools' formatting to be predictable/unsurprising, but I don't think that applies to character-precision). By analogy, if we were specifying a drawing tool, we would want the output of the tool to be roughly recreatable by an artist, but we would not expect the output to be a pixel-precise match to the artist's output.
In practical terms, I think it is fine for the spec to be written in terms of both hard limits and recommendations - for example, for function calls the hard limit would be 'fits in the max line length' and the recommendation is something like 'functions may be spread over multiple lines if that makes reading easier' where a person interprets that how they like, and Rustfmt interprets that as > 60 chars.

@nrc I would probably split that function call as well, but not because of length.  If the last argument didn't consist of a multi-argument function call itself, I'd write that on one line.  As written, I'd wrap it to avoid making people skimming the line have to deal with two levels of comma-separated arguments on the same line.

On a different note, I'd prefer to define "short" as an inclusive length (for instance, a struct literal including the structure name and braces) rather than exclusive (excluding the structure name as braces).

I would probably split that function call as well, but not because of length

I probably would split too, however, Rustfmt can't really do that so we have to make the most of what we have.

On a different note, I'd prefer to define "short" as an inclusive length (for instance, a struct literal including the structure name and braces) rather than exclusive (excluding the structure name as braces).

Can you explain why please?
For me, the motivation here is purely to make the arguments more readable, so I wouldn't want to split a_long_object.a_very_long_method_name(x, y, z) but I would want to split x.m(a_long_argument, another_long_argument) (waving hands about the actual lengths, and actually, manually, I would not actually split the second example because the arguments are not complex).

I probably would split too, however, Rustfmt can't really do that so we have to make the most of what we have.

Rustfmt could apply the heuristic I described: "split an outer function call's arguments onto one line each if one of them contains an inner function call with multiple arguments".  Not perfect (println!("{}", foo(a, b)); doesn't really need wrapping), but a decent approximation of the actual human heuristic.  Split lines based on complexity and ease/speed of mental parsing, not length.

Copying some discussion here from other issues: I think I'd really prefer to define "simple" rather than "short".  For instance, in the context of an enum struct variant, it doesn't matter to me how many fields the struct has (as long as they fit on one line), but as soon as a field has a non-trivial type, an attribute, or anything other than name: SingleTokenType, I'd want to break the fields onto separate lines, no matter how few characters they fit into.

Quick summary of the three alternatives discussed in today's style meeting (which we didn't decide on yet, but which we characterized as a refresher):

Width (more than N characters)
Number of tokens (more than N arguments)
Complexity of expressions (individual components more than one simple token, so a, b, c, d, e, f, g would be fine but a + b(c) would not)

To me, I prefer to break things into multiple lines with appropriate indentation and alignment ANY time the complexity exceeds a certain "noisiness". This is a rather subjective idea that is determined by: Number of Arguments/Well-Defined Potential Split Points, Overall Length if not split, Longest Length of segments if split at well-defined potential split points.
Generally speaking, if there are 3 or more well-defined potential splits OR 2 or more potential splits and at least one of them is a significant fraction of the preferred remaining line length (after deducting already indented amount), OR the overall line length is greater than 80% of the preferred line length and there is at least 1 well-defined potential split point - In all of these cases, I would seek to split and indent/align along all the potential split points. Each split segment, would then have similar evaluation done on it for the chance for itself to also be split to further lines using similar rules.
What do I mean by potential well-defined split points? Commas between arguments of method calls, method definitions, struct elements, enum elements, array elements in initializers, "." between method invocations in chained method invocations, etc.

We just discussed this in the rust-style meeting, and I think we're very close to a consensus. Here's some summaries of the current state of the discussion:
We're currently entertaining two major possibilities.
Possibility 1: a width metric:

if what's between the braces/parens is longer than N characters, for some N, break each argument/param to its own line.

Possibility 2: a simplicity-based metric. Current straw proposal:

No more than 4 arguments or 2-3 name: value fields
Each field is "simple": no more than a single identifier or literal constant, with unary operators (&, *) and numeric suffixes (u32)

In evaluating these, there are four interesting cases to consider:

short and simple
long and complex
long and simple
short and complex

Both of these proposals handle "short and simple" and "long and complex" just fine. They differ on "long and simple" or "short and complex":

A width-based metric would break "long and simple" cases across lines, such as a handful of simple field: value pairs, where the field names and values used long, descriptive identifiers.

A width-based metric would leave "short and complex" cases on one line. That could allow for quite complex expressions as long as they fit in the line limit; we managed to come up with some uncomfortably complicated expressions that fit inside 40-50 characters, which we felt like ought to be broken across lines.

A simplicity-based metric would leave "long and simple" cases alone, even if things like descriptive identifiers pushed them over the value of N.

A simplicity-based metric would break "short and complex" cases across multiple lines.

We had a general consensus that we shouldn't build a set of rules that included both complexity and a numeric width limit; that made the rules too hard to explain and evaluate in practice.  We should pick one or the other.
Note: the selection of "4 arguments" or "2-3 fields" was based in part on things like (x, y, z, w) vectors, as used in graphics.  We originally said "4 arguments/fields", but we felt that name: value was more complex than value alone.
We'd like to make sure the "simplicity" rules are as simple as possible. The straw proposal above seems like it might work; if it grew multiple additional rules or corner cases, that'd make it less appealing.
We'd like to evaluate both of the proposals above (numeric width, or simplicity), and figure out which one produces more satisfying results.

but we felt that name: value was more complex than value alone.

So, would that be affected by struct field shorthand?
let (foo, bar) = ...;
Foo { foo, bar, baz: 65 }

I could see an argument that a shorthand field is only worth 0.5 of a full struct field for the simplicity-metric.

@Nemo157 Yes, you could have up to 4 (simple) fields if you're using the struct field shorthand.
Thanks for pointing out that you can mix the two, though. I think the rule I'd suggest is "up to 4 arguments or shorthand fields, or up to 3 fields if any of them are name: value pairs".

In the interests of making some forward progress here: would someone be willing to try implementing the proposed strategy in rustfmt and posting some sample code formatted with it, so we can see how the results look on real code?

would someone be willing to try implementing the proposed strategies in rustfmt

I will do so, but won't have time in the next week or so. Not sure if it is something that @topecongiro is interested in working on?

@nrc I will work on it sometime this week.

I will work on it sometime this week.

Thank you!

Three different styles (they only apply to function calls and alike for now) and examples (diffs between each style and width-based strategy in rustfmt code base) for comparison.

No short rule: allow single line as long as it does not exceed max width (examples).
Simplicity-based: allow single line when there are no more than 4 arguments (examples).
Mixed: allow single line when width-based or simplicity-based rules hold (examples).

@topecongiro Thank you very much for working on this!
Minor nit: the handling of trailing commas needs some work. In a couple of cases, lines contained a trailing comma followed by a close paren. Search for ,) to find examples. Conversely, in the simplicity case, several lines that should have trailing commas (the last argument of a function, for instance) don't.
Much more significant issue: the rule isn't "no more than 4 arguments", it's "no more than 4 simple arguments", for the definition of simplicity mentioned in https://github.com/rust-lang-nursery/fmt-rfcs/issues/47#issuecomment-314277255 :

No more than 4 arguments (or shorthand fields) or 3 name: value fields
Each field is "simple": no more than a single identifier or literal constant, with unary operators (&, *) and numeric suffixes (u32)

(It also looks like this got applied to match patterns, and for that case, I think I'd also add ref and ref mut to the permitted unary operators.)
All that aside, the examples you provided still provide a great deal of help in evaluating concrete cases.  I'm going to post a follow-up comment discussing specific examples and the desirability of their formatting.

From the "mixed" examples:
-        WriteMode::Diff => {
-            if let Ok((ori, fmt)) = source_and_formatted_text(text, filename, config) {
-                let mismatch = make_diff(&ori, &fmt, 3);
-                let has_diff = !mismatch.is_empty();
-                print_diff(mismatch, |line_num| {
-                    format!("Diff in {} at line {}:", filename, line_num)
-                });
-                return Ok(has_diff);
-            }
-        }
+        WriteMode::Diff => if let Ok((ori, fmt)) =
+            source_and_formatted_text(text, filename, config)
+        {
+            let mismatch = make_diff(&ori, &fmt, 3);
+            let has_diff = !mismatch.is_empty();
+            print_diff(mismatch, |line_num| format!("Diff in {} at line {}:", filename, line_num));
+            return Ok(has_diff);
+        },

This seems very wrong. Breaking in the middle of the if let is much worse than breaking before it and leaving the whole if let on one line. It should not have been combined. What rule allowed this?
From the "none" examples:
     let mut rewrites = try_opt!(
-        iter.map(|(e, shape)| {
-            rewrite_chain_subexpr(e, total_span, context, shape)
-        }).collect::<Option<Vec<_>>>()
+        iter.map(|(e, shape)| { rewrite_chain_subexpr(e, total_span, context, shape) })
+            .collect::<Option<Vec<_>>>()
     );

The old way looks better here. In practice I'd probably write this as:
     let mut rewrites = try_opt!(iter.map(|(e, shape)| {
-            rewrite_chain_subexpr(e, total_span, context, shape)
-    }).collect::<Option<Vec<_>>>());

But I don't think that generalizes, because the trailing collect seems problematic if you stack a pile of other closings on it. So I can live without that if we can't come with a general rule that allows it.  (Also, does replacing try_opt! with trailing ? make that better?)

I mostly agree with those observations. Some comments

This seems wrong. I'd write that on one line. token.token doesn't seem excessively complex.

One thing we didn't get to last week was whether 'short' means the same thing in all some circumstances. Currently it does not. I think that makes sense - people have a higher tolerance for single-line complexity in function calls than struct literals, I think.
So, we could probably allow the complexity measure to be more complex in function calls (I have to say though that this level of complexity in the spec is making me really uncomfortable).

This is an improvement. I agree with formatting the format_tuple_struct call on one line here.

I agree, but I don't see how this comes out of our rules, it looks like a lot (>4) simple args.

I feel like there's a very clear general rule we're missing here that would fit all the above examples I quoted. Anyone up for trying to help articulate it?
I think it has something to do with "spreading complexity across lines" rather than packing complexity into one line. Sometimes we have multiple potential break-points on one line. The "combining openings and closings" rules are an effort to decide how to handle that. So are these rules for splitting arguments or fields across lines. So the question is, how do we handle multiple splits?  That's separate from "how do we decide whether to split", but I feel like the answers may be closely related.  In both cases, we're trying to spread out complexity.

@nrc Well, we could reasonably ask whether we want to break function calls that don't exceed the line length limit. That's separate from "if we're breaking it, where and how do we break, if we have multiple choices".

@joshtriplett

Minor nit: the handling of trailing commas needs some work. In a couple of cases, lines contained a trailing comma followed by a close paren. Search for ,) to find examples. Conversely, in the simplicity case, several lines that should have trailing commas (the last argument of a function, for instance) don't.

This is because rustfmt preserves the presence of trailing comma inside macro since adding or removing it might cause the code not to compile.

Much more significant issue: the rule isn't "no more than 4 arguments", it's "no more than 4 simple arguments", for the definition of simplicity mentioned in #47 (comment) :

Just in case you are interested, rustfmt currenlty treats arguments without newlines as 'simple'.

Finally, I see several cases under the Simplicity examples that meet the criteria but got broken across lines. For instance:
-        rewrite_struct_field(context, self, shape, prefix_max_width)
+        rewrite_struct_field(
+            context,
+            self,
+            shape,
+            prefix_max_width,
+        )

Why did this get rewritten? It should use the one-line form.

This is caused by a typo 😞. args.len() > 3 should be args.len() > 4.
    let tactic = match context.config.fn_call_one_line_style() {
        OneLineStyle::Simplicity if args.len() > 3 => DefinitiveListTactic::Vertical,
        _ => definitive_tactic_for_args,
    };

Well, we could reasonably ask whether we want to break function calls that don't exceed the line length limit. That's separate from "if we're breaking it, where and how do we break, if we have multiple choices".

Indeed. I think we have three choices:

don't break function calls early
break function calls early with the same criteria as struct lits
break function calls early with different criteria

Rustfmt currently does the last. Heuristically, I have found this most closely matches existing code which I think looks good.

I think of something as short if it fits in one line. That is, depending on your maximum line length:
// max line length -------|
// Short:
let x = Foo { x: 42 };
// Long:
let x_is_too_long = Foo {
    x: 42
};

// Short
struct Foo { x: usize }
// Long
struct FooToo {
    xLong: usize
}

// Short:
fn f(x: u32) -> u32 { 42 }
// Long:
fn foo(xoo: u32) -> u32 {
    42
}

Clang format has independent knobs to allow short "if clauses", "functions", "for loops", etc. But the definition of short there is consistent: if the X fits in one line, then it will be put in one line. Whether X is a loop or a class/function declaration is orthogonal to the definition of short.

If we do go with the complexity scheme, I feel like we need to consider very small numbers of arguments. In particular, istm that if there is a single argument, then a function call should never get multi-lined (unless it hits the max line width). I don't think that should apply to struct lits though. Also worth considering a function call with two fairly complex arguments, where we would probably put that on one line if hand-writing.

@nrc I agree regarding single arguments, and honestly, I think that applies to single fields, too. And honestly, the more I look at the examples, the more I'm inclined to err on the side of "keep it on one line if it fits", at least for functions.

If I may throw my hat in the ring, for my taste current rustfmt does not value vertical density enough. Here are some examples of changes I disliked:
diff --git a/src/client/mod.rs b/src/client/mod.rs
index b8ff658..41db84c 100644
--- a/src/client/mod.rs
+++ b/src/client/mod.rs
@@ -32,7 +32,10 @@ impl Client {
     }

     pub fn call(self, cmd: Command) -> ResponseStream {
-        let Client { transport, mut state } = self;
+        let Client {
+            transport,
+            mut state,
+        } = self;
         let request_id = state.request_ids.next().unwrap();
         let (cmd_bytes, next_state) = cmd.to_parts();
         let future = transport.send(Request(request_id.clone(), cmd_bytes));

@@ -155,11 +169,17 @@ pub struct FetchCommand {
     fn prepare(self) -> FetchCommand;
     fn build(self) -> Command {
         let FetchCommand { args } = self.prepare();
-        Command { args, next_state: None }
+        Command {
+            args,
+            next_state: None,
+        }
     }
     fn changed_since(self, seq: u64) -> FetchCommand {
         let FetchCommand { mut args } = self.prepare();

@djc Agreed in both cases.

my 'short': not exceed the max-column-count
my 'long': exceed the max-column-count
'short' <= 'long'
(max-column-count is a customized const value, maybe 80, 100, or 110. I like a bigger number, since many of us have big monitor.)

We discussed this again at the meeting this week. We covered a bit of ground about what we liked and didn't like, looked at some examples, and discussed the motivation for such heuristics.
We still didn't agree exactly on a definition, but in order to move forward I propose the following:

we agree we should have some heuristic
we agree to specify a function signature for an is_simple function or functions which provide an answer to the question 'should this list be formatted on a single line or across multiple lines?'.
we should decide how the implementation of these function(s) should be decided:

we could leave it up to implementers
we could specify it precisely (I prefer to avoid this)
we could punt for a future RFC (i.e., leave it unspecified in the first draft)
we could provide a suggestion/guideline and leave it technically unspecified
something else

we collect examples of code which we think looks good or bad to motivate this discussion (in particular examples from previous attempts to do this which have gone 'wrong').

I propose this as a necessary but not sufficient requirement for an item to be short:

contains at most one statement, i.e. one ;.

Based on extensive discussions both within the style team and elsewhere, I think I might have a rough idea (needing refinement) for criteria for "simple", and for how we should break lines.

If something contains binary operators at at most one level, such that it's trivial to mentally parse what operands appear at what level, then it's "simple". (It might still need breaking across lines if it exceeds the line length limit.)
If something contains binary operators at more than one level, such that figuring out which level any given operand sits at requires careful mental parsing of parentheses, then it isn't "simple". This includes expressions separated by commas (e.g. tuples as arguments), or structures as arguments.

For example, a call like f(a, b, c, d, e, f, g, h, i, j) is "simple", while a call f((a, b), (c, d)) is not, because in the latter (especially as the expression grows more complex), reconstructing the parse tree in your head isn't trivial. It breaks fast mental parsing heuristics.
Something like f(g(h(a + b + c))) is also "simple" by this metric. But f(g(h(a + b) + c)) and f(g(h(a + b), c)) are not.

If something contains trailing unary operators (such as ?) or trailing chained function calls (such as .unwrap()) and has closing parentheses both inside and outside those, that isn't "simple" either, because it would require carefully matching up the closing parentheses to see what level that operator applies at. For instance, anything ending in )))?)) isn't going to be "simple". (This needs to be stated more precisely.)

As a rough sketch, how does that sound? I think this would capture many aspects of the heuristic we're subconsciously using to say "that needs breaking across lines".

A couple of additional thoughts, though I'm less certain about these:

For the purposes of the above, a nested function call can still be "simple" if it doesn't contain multiple arguments at multiple levels. For instance, foo(bar(baz(a, b, c, d))) and foo(bar(a), baz(b)) are both still "simple", because the whitespace naturally groups the arguments and a fast mental parse produces the correct result. However, foo(bar(baz(a, b), c), d) is not "simple", because a fast mental parse has to carefully consider parentheses and commas to associate the arguments with their functions.
Similarly, a trailing function call alone can still be "simple" if it doesn't contain trailing function calls at multiple levels of parentheses. For instance, func(thing.foo(), thing2.bar()) is still "simple", because a fast mental parse glues the trailing functions to the names they apply to, due to the lack of whitespace around the . of method invocation. However, func(thing.foo(), thing2.bar()).baz() and even func(thing2.bar()).baz() are not "simple", because those same mental parsing heuristics don't make it trivial to account for the ) that separates thing2.bar() from .baz().

If something contains binary operators at more than one level, such that figuring out which level any given operand sits at requires careful mental parsing of parentheses, then it isn't "simple".

I don't know how that is meant to be defined but while this expression is arguably not simple:
add(mul(a, b), mul(c, d))
the following certainty is, imho:
a * b + c * d
as well as this one:
(a + b) * (c + d)

@repax I've been talking about this primarily in the context of things like function calls and struct initializers, because those are where we invoke the concept of "short"/"simple". We don't currently apply those criteria to expressions like those. They could form a part of a larger expression to which we would, though, so we do have to think about them a little.
I personally tend to err on the side of adding unnecessary parentheses and almost never relying on operator precedence, just to simplify mental parsing. But in general, I agree that both of the expressions you posted seem simple. (They could become non-simple if applied to more than just a, b, c, and d.)
I think the premise I'd use for that particular point is that thanks to precedence, we can think of a * b + c * d and (a + b) * (c + d) each as "one" expression, with a series of binary operators, which we mentally parse using infix operator precedence rules. That doesn't have the same problem that, for instance, f(g(h(a + b) + c)) has, where you have to switch back and forth between prefix and infix to parse it. You can see, without too much difficulty, that a + b comes at a different level than  + c, but then you have to carefully match parentheses to see that it goes with h. And while that looks trivial in that case, when those names and argument lists get longer, matching up the parentheses becomes the main problem in fast mental parsing.

I think I prefer to leave this basically unspecified - looking at the amount of detail required in @joshtriplett 's suggestion, I think it is better to leave it entirely up to an implementer, and only say that implementers may have a concept of short or simple defined in some way.

I'm feeling similar to @nrc

@nrc To be clear, I don't think this level of detail is required to actually specify what "short" means. I've been carefully enumerating and classifying cases, but I feel like there's a simple set of rules to extract from those cases that would capture the meaning. I'd appreciate some help capturing those rules, and then testing them against the various cases to see if they produce the same results.
By "leave it entirely up to an implementer", do you mean that rustfmt might still do this but we wouldn't put it in the style guide? What's the advantage of doing it that way? I'm concerned that if we do that, we'll get noticeable inconsistencies in this area, if there are multiple tools that format Rust.

By "leave it entirely up to an implementer", do you mean that rustfmt might still do this but we wouldn't put it in the style guide? What's the advantage of doing it that way? I'm concerned that if we do that, we'll get noticeable inconsistencies in this area, if there are multiple tools that format Rust.

The advantage of not spec'ing:

allows implementations to incrementally improve their design
it makes the spec simpler
it makes implementation easier (lowering the bar for more implementations)
it is less effort (better to ship something slightly sub-optimal than to never ship the perfect spec)

So, I think for any issue there is a trade-off between specification complexity and the effect of the specification. My belief is that the downsides of not spec'ing this (i.e., the variation between implementations) do not outweigh the benefits here - I think that any variation in this respect will not be very noticeable and that there will be far wider variations from other design decisions. (Also, practically speaking - there is only one implementation and I'd rather experiment with this question in code rather than trying to get the design perfect up front).


Misleading statement on authentication

Line 305 ff in IUA.b, Rev.1.2 from September 21, 2019, states:
The SAML Token Option enables integration of environments that use both SAML identity federation and OAuth authorization infrastructure. This enables the end user to control authorization of applications through OAuth when the user identity authentication is already provided through SAML identity federation.
The statement is not correct: the IUA Authorization Server responds a authorization or access token, which is JWT or - in the SAML option- SAML 2.0. It may include identity claims, but it is no user identity assertion.

refactored the relevant parts as discussed during the July F2F meeting.


Feature:  Suggest alternative approach to providing connection information to indy ledger

The aries cloud agent is architected to extract the IP address of the nodes in the indy ledger from the genesis transactions for the ledger. If I am not mistaken this may be problematic in the future.  A given indy ledger may over time have a complete turnover in the nodes hosting the ledger and hence not share any IP addresses with the nodes that formed the genesis transactions. Given that the architecture choice of using the genesis transactions to bootstrap communications with the ledger is pervasive throughout the aca python design and shows up in numerous places in terms of command options etc. It seems that this might be a large latent problem and fixing it sooner rather than later would be prudent.
Some suggestions. The current approach of using a web server / ledger browser as a mechanism for extracting information from a pool is one viable path. But as it currently stands even this browser requires (von-network) the ledger transactions in order to talk to the ledger.
A public indy ledger should have a WELL KNOWN URL that provides the node IP addresses. This should be the architecture used for public ledger. In development environments or otherwise private ledgers a list of current node ip addresses should be trivially available and all agents either use that list or bootstrap by tasing to a ledger browser that provides that. But the IP connection should not be via the genesis transactions.
Maybe I am missing something.

I agree with the thought here, but I think it is an indy-node issue, not an aca-py issue.  Once a better mechanism is supported in indy-node for contacting the ledger from a genesis file, we can support it in aca-py.
Some additional points:

the use of a URL for retrieving the genesis file works for other than von-network. For example, the Sovrin ledgers post their genesis files on github so that they can fetched via a URL.
The "staging" (formerly known as test) Sovrin network recently had this issue that the handful of original IPs referenced in the genesis file were removed from the pool. I believe the solution they came to was to add all stewards to the genesis file. The file is available here: https://raw.githubusercontent.com/sovrin-foundation/sovrin/stable/sovrin/pool_transactions_sandbox_genesis

I don't see anything that should be done by ACA-Py specifically.  The genesis file is available in various forms depending on the use case - generally split between developer ledgers and more permanent ledgers and ACA-Py supports the different use cases.  We might want to add more docs around the use cases, but I don't think the functionality needs to change.
Will close this barring further comments/suggestions.
Thanks!

I was missing something and that is the practice of adding all current nodes to the genesis file. So this is not a latent issue if indeed that practice continues.  I think then the one thing that ACA can do is support a local copy of the genesis file via a file path  not the contents of the file.

Sounds good.  We have another issue for supporting adding the --genesis_file <file> option.
Closing this issue.

I agree although genesis file is now a bad name if it includes more that the genesis nodes

That’s up to the Indy guys to address


[QoL] String data formatting

I don't like the "... at the end of the first String line.
I would like to be able to format string data so I can see all of the string data.

Ah, I just found that widening the column showed all the data eventually. Sorry for the noise.

FWIW, I think there's still an opportunity to make this better by making the line-wrap point of the Operand field configurable.  Right now you have to choose between "..." in your operand and having Comment way over to the right.  Right now it's fixed at 64 chars.
It requires re-flowing all of the strings in the project because it potentially changes the number of lines, so it's not something you'd want to do dynamically as the column is resized, but it could easily be an app setting.


[1.2] Mode Base + derivations - invalid StartUpMode test (TC 3.2)

Reproduction steps
Step 10 of Mode Base (and derived cluster) test plan expects the device's current mode to be equal to that of the startup mode attribute, after a device power cycle. This is incorrect if OnMode is not null, because in that case it overrides StartUp Mode
"StartUp Mode - If this attribute is not null, the CurrentMode attribute SHALL be set to the StartUpMode value, when the server is powered up, except in the case when the OnMode attribute overrides the StartUpMode attribute "
"6.5.1. OnMode with Power Up
If the On/Off feature is supported and the On/Off cluster attribute [StartUpOnOff] is present, with a value of On (turn on at power up), then the CurrentMode attribute SHALL be set to the OnMode attribute value when the server is supplied with power, except if the OnMode attribute is null."
The TC must include a precondition so that the StartUpMode will not be overridden by the OnMode attribute.
Note: the same issue appears to affect the ModeSelect TP.
Bug prevalence
100%
GitHub hash of the SDK that was being used
732f797
Platform
core
Platform Version(s)
No response
Type
Test Improvement
Anything else?
No response

@plauric Generally test plan issues should get filed in https://github.com/CHIP-Specifications/chip-test-plans/

@bzbarsky-apple Indeed. I'll close this one and will track this issue in the TP repo: https://github.com/CHIP-Specifications/chip-test-plans/issues/3296


merge scripts updates

:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

BingAds/scripts/reference/Account.md
:white_check_mark:Succeeded

BingAds/scripts/reference/AccountsApp.md
:white_check_mark:Succeeded

BingAds/scripts/reference/ExecutionResult.md
:white_check_mark:Succeeded

BingAds/scripts/reference/HTTPResponse.md
:white_check_mark:Succeeded

BingAds/scripts/reference/Keyword.md
:white_check_mark:Succeeded

BingAds/scripts/reference/UrlFetchApp.md
:white_check_mark:Succeeded

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


bugfix: Unit test often time out

What is the purpose of the change
see #8438
Brief changelog

Release Zookeeper resource
Remove unused import

Verifying this change

Checklist

[x] Make sure there is a GitHub_issue field for the change (usually before you start working on it). Trivial changes like typos do not require a GitHub issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
[x] Each commit in the pull request should have a meaningful subject line and body.
[x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
[x] Check if is necessary to patch to Dubbo 3 if you are work on Dubbo 2.7
[x] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add sample in dubbo samples project.
[ ] Add some description to dubbo-website project if you are requesting to add a feature.
[x] GitHub Actions works fine on your own branch.
[ ] If this contribution is large, please follow the Software Donation Guide.

Codecov Report

Merging #8439 (278dd01) into 3.0 (2f099bd) will decrease coverage by 0.04%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                3.0    #8439      +/-   ##
============================================
- Coverage     63.19%   63.14%   -0.05%
- Complexity      310      311       +1
============================================
  Files          1103     1103
  Lines         46230    46230
  Branches       6978     6957      -21
============================================
- Hits          29215    29194      -21
- Misses        13729    13760      +31
+ Partials       3286     3276      -10

Impacted Files
Coverage Δ

...ng/transport/dispatcher/all/AllChannelHandler.java
62.06% <0.00%> (-20.69%)
:arrow_down:

...dubbo/common/status/support/LoadStatusChecker.java
46.15% <0.00%> (-15.39%)
:arrow_down:

...ng/transport/dispatcher/WrappedChannelHandler.java
43.47% <0.00%> (-15.22%)
:arrow_down:

...in/java/org/apache/dubbo/common/utils/JVMUtil.java
81.13% <0.00%> (-11.33%)
:arrow_down:

.../org/apache/dubbo/remoting/ExecutionException.java
15.78% <0.00%> (-5.27%)
:arrow_down:

...nt/metadata/StandardMetadataServiceURLBuilder.java
85.36% <0.00%> (-4.88%)
:arrow_down:

...ster/router/condition/config/ListenableRouter.java
26.82% <0.00%> (-4.88%)
:arrow_down:

.../remoting/transport/netty4/NettyServerHandler.java
62.79% <0.00%> (-4.66%)
:arrow_down:

...ubbo/rpc/protocol/dubbo/ChannelWrappedInvoker.java
43.47% <0.00%> (-4.35%)
:arrow_down:

...etadata/report/support/AbstractMetadataReport.java
57.89% <0.00%> (-3.83%)
:arrow_down:

... and 29 more

Continue to review full report at Codecov.

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


The set-output and save-statecommand is deprecated and will be disabled soon

See https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
It will be disabled from 23rd March 2023

Already handled in https://github.com/erlef/setup-beam/pull/148

Yes, this should be in v1.14, already. If you're using v.13, or v.13.x you won't have it, though. Thanks.


lpcxpresso55s69_cpu0 samples/subsys/usb/dfu/sample.usb.dfu build failure

Describe the bug
Build failure for:
west build -p -b lpcxpresso55s69_cpu0 samples/subsys/usb/dfu
fails with
/zephyrproject/bootloader/mcuboot/boot/bootutil/zephyr/../include/bootutil/bootutil_public.h:86:1: error: static assertion failed: "Unsupported value for MCUBOOT_BOOT_MAX_ALIGN"
   86 | _Static_assert(MCUBOOT_BOOT_MAX_ALIGN >= 8 && MCUBOOT_BOOT_MAX_ALIGN <= 32,
      | ^~~~~~~~~~~~~~

The check was introduced in
95292e85aa manifest: mcuboot update
@nvlsianpu can you take a look?
This is the only error in the last full run https://github.com/zephyrproject-rtos/zephyr/runs/8482669051
To Reproduce
west build -p -b lpcxpresso55s69_cpu0 samples/subsys/usb/dfu on current main.
Expected behavior
Builds
Impact
CI failure

(@stephanosio fyi, from the yesterday's full run)

This platform should be excluded from the test:

Up to 640 KB on-chip flash program memory with flash accelerator and 512 byte page erase and write.

mcuboot cannot work on this device

@dleach02 seems like the various lpc55 platforms cannot support mcuboot right now due to limitations on the page write size:
dts/arm/nxp/nxp_lpc55S0x_common.dtsi:                   write-block-size = <512>;
dts/arm/nxp/nxp_lpc55S1x_common.dtsi:                   write-block-size = <512>;
dts/arm/nxp/nxp_lpc55S2x_common.dtsi:                   write-block-size = <512>;
dts/arm/nxp/nxp_lpc55S3x_common.dtsi:                   write-block-size = <512>;
dts/arm/nxp/nxp_lpc55S6x_common.dtsi:                   write-block-size = <512>;

See:
https://github.com/zephyrproject-rtos/mcuboot/blob/2fcae1c1a05fe3a0892ba3c649f93cf3e1077949/boot/bootutil/include/bootutil/bootutil_public.h#L86-L87
I'll send a PR to exclude the board from the test but maybe it should not have the mcuboot partitions defined in the first place if it's not supported:
https://github.com/zephyrproject-rtos/zephyr/blob/1bbe209a30fcdab037c8ac9b96c33ea2249665e0/boards/arm/lpcxpresso55s69/lpcxpresso55s69.dtsi#L120-L140
Is there' something I'm missing?
cc @mmahadevan108


trollface test :trollface:

DO NOT REMOVE OR SKIP THE ISSUE TEMPLATE

[X] I understand that I will be blocked if I remove or skip any mandatory* field

Checklist

[X] I'm reporting a broken site
[X] I've verified that I'm running yt-dlp version 2022.08.19 (update instructions) or later (specify commit)
[X] I've checked that all provided URLs are playable in a browser with the same IP and same login details
[X] I've checked that all URLs and arguments with special characters are properly quoted or escaped
[X] I've searched the bugtracker for similar issues including closed ones. DO NOT post duplicates
[X] I've read the guidelines for opening an issue
[X] I've read about sharing account credentials and I'm willing to share it if required

Region
(you can test some emoji here for a while)
Provide a description that is worded well enough to be understood
:trollface:
Provide verbose output that clearly demonstrates the problem

[X] Run your yt-dlp command with -vU flag added (yt-dlp -vU <your command line>)
[X] Copy the WHOLE output (starting with [debug] Command-line config) and insert it below

Complete Verbose Output
$ ytdl -v :trollface:
[debug] Command-line config: ['-v', ':trollface:']
[debug] User config "/home/lesmi/.config/ytdl-patched/config": ['--cookies=$HOME/ck.txt', '-i', '--escape-long-names', '--throttled-rate', '100k', '--remux', 'f4v>mkv/mov>mkv/unknown_video>mkv', '--no-lock', '--reject-title', '(?i)TikTok|Tinder']
[debug] Encodings: locale UTF-8, fs utf-8, pref UTF-8, out utf-8, error utf-8, screen utf-8
[debug] ytdl-patched version 2<IP_ADDRESS>6466116  (homebrew)
[debug]      from commit a49623d95
[debug]      based on 2c60eae89
[debug] ** The command you are running is not yt-dlp.
[debug] ** Please make bug reports at https://github.com/ytdl-patched/ytdl-patched/issues/new/choose instead.
[debug] Lazy loading extractors is disabled
[debug] Python 3.9.13 (CPython 64bit) - Linux-5.15.0-33-generic-x86_64-with-glibc2.35 (glibc 2.35)
[debug] Checking exe version: ffmpeg -bsfs
[debug] Checking exe version: ffprobe -bsfs
[debug] exe versions: ffmpeg 4.4.2 (setts), ffprobe 4.4.2, rtmpdump 2.4
[debug] Optional libraries: Cryptodome-3.14.1, WebSocketClientWrapper-None, brotli-1.0.9, certifi-2021.05.30, mutagen-1.45.1, sqlite3-2.6.0, websockets-10.2
[debug] Proxy map: {}
[debug] [generic] Extracting URL: :trollface:
ERROR: [generic] ':trollface:' is not a valid URL. Set --default-search "ytsearch" (or run  yt-dlp "ytsearch::trollface:" ) to search YouTube
  File "/home/linuxbrew/.linuxbrew/Cellar/ytdl-patched/1656466116/libexec/lib/python3.9/site-packages/yt_dlp/extractor/common.py", line 671, in extract
    ie_result = self._real_extract(url)
  File "/home/linuxbrew/.linuxbrew/Cellar/ytdl-patched/1656466116/libexec/lib/python3.9/site-packages/yt_dlp/extractor/generic.py", line 2844, in _real_extract
    raise ExtractorError(

same issue here :trollface:

:troll: :troll:

I understand that I will be blocked if I remove or skip any mandatory* field

hypocrisy is the worst of human traits :trollface:

:trollface:


Various tweaks to command argument handling

[replacing #784]
This PR covers a few long-standing issues about command argument format and hopefully command argument handling, as well as refactoring some annoying points of using CommandSpec and CommandElements.
The ones I want to get are:

[x] #604
[x] #583
[x] Adding more tags to the various location-related command arguments
[x] #689 -- An event that is called for chat as well as commands (very preprocessey) -- completed, this is covered by #957
[x] #780
[x] Only parse escapes for characters that would otherwise have a special function
[ ] Refactor CommandArgs + CommandContext to be less dumb (see PEX wip on this (though what happens from this will be different))
[ ] #924
[x] #989

notable todos:

Implement #spawn:<world> on LocationCommandElement
Properly handle tabcompletion on LocationCommandElement

I have no idea if it was already fixed in one of PRs, but...
When command with subcommands is inputted by user incorrectly, currently it displays usage of main command (I.e. only list of subcommands).
For example, command "mycmd" have subcommand "change", which in turn have subcommand "coords". User inputs "/mycmd change argblarg":

It would be more useful if error would display list of valid child commands of "change" (coords, etc), not a list of child commands of the root command.
User inputs "/mycmd change coords too many a r g u m e n t s":

Error should display usage of the end command "/mycmd change coords" (I.e. it's parameters, etc).

While you're changing things, I'm not sure I like the cc style error indicator.
Given the (unfortunate) variable-spaced font, I think it would be better to highlight the broken section with a pink? colour. It's really ambiguous where the ^ points to sometimes.

A lot of your lambdas have two spaces before the param list.

@dualspiral
Closing this as this should be handled, if necessary, in the commands PR.


runplus: only prompt to resume after a given duration

For example, when doing reps with 1 minute rests, a user can configure runplus so it automatically resumes if started within the minute

@halemmerich @thyttan does this seem ok to you both? I think you're the main users of runplus

I'm not quite sure what you mean by this. Is it auto resume after manually stopping for a slow/rest period? The target would be to have only the high intensity periods tracked/shown?

Yeah exactly, like if you're doing reps with a lot of standing about in between - I found I'd have to press start then wait and tap the resume button, sometimes forgetting during a session

By automatically, it's not that it sets a timer and starts by itself after i runs out? Just that the menu is not displayed within the window of the timeout and the records will be appended to the last log?

Yeah that's poor choice of phrasing on my part - it's exactly the existing behaviour, just with the ability to customise the time period (so like you say, currently the user is always prompted to resume the run or start anew after 10s)

How about letting a short button press pause and resume (including showing paused as state in the bottom right) and longer button press stop the recording? That way there is no additional setting and it is always possible to actually end the run. It would allow more flexible intervals.
Maybe even write a line into the recorder log when the pause starts and ends instead of actually pausing the recorder to make later analysis easier. That would allow to check for pulse recovery etc. during the rest intervals.
Thinking about it, maybe short press could just signify a new interval/round start/stop and otherwise everything would just run like normal? The UI could do whatever it wants with the intervals, i.e. reset exstats, show averages/values only for the last interval or whatever.

How does actually stopping work? Say I configure a 5 minute auto resume time, do I have to kill runplus and go to the recorder to stop the run?

For a 5 minute resume time, yes, you'd have to restart runplus - but that'd be it, it would then kick off a new recording (in fact you'd get a prompt - see below snippets), you wouldn't need to tangle with the recorder app manually:
https://github.com/espruino/BangleApps/blob/e856e381fad97cea0c88acd5cbb78c448c1abe0b/apps/runplus/app.js#L84-L91
https://github.com/espruino/BangleApps/blob/e856e381fad97cea0c88acd5cbb78c448c1abe0b/apps/recorder/widget.js#L267-L273

So if a later rework to include laps or other handling of the start/stop/pause/resume mechanics is still on the table I'm fine with this change. Maybe we should signify if the timer has run out in the UI by replacing the STOP by PAUSE or something like that as long as it would autoresume on button press.

Sounds good, sorted the timer UI (dbc9833), let me know what you think

I've played around with this more and I think the timeout isn't worth the complexity, my use case would be to have the timeout be ridiculously high anyway - I've changed it to a simple toggle.
Given this a test and it looks to work well, let me know what you think :)

Thanks!

Thanks for the review/merge!


feat: add the possibility of filtering the diff by content_type

Summary
This PR introduces a new flag --content-type to the create command. The --content-type flag is optional and allows users to specify a content type id when creating a new changeset.
Changes

Added --content-type flag to create command in src/commands/create/index.ts.
The BaseContext now includes contentType which is set from the --content-type flag.

This change allows users to have more control over the changesets they create by specifying a content type id.

Hey @marcolink
I've changed the content-type filter to query-entries :)
I've also added a new parameter, allowed-operations.
I needed to create a patch with only add operations, so I added this new options with the default operations as default value.
I'll edit the main description/title of this PR.

Bumping this PR as this is something that is extremely useful. I sincerely believe this is the single feature that is missing to make this project stand out.
At the current state, contentful-merge only computes differences between all the entries between two environments but it is almost always more practical for us to just delete one env and recreate it from the other that using what this repo offers.
This PR does what we were forced to make internally (as a fork of this repo) in order for us to migrate specific pages between our dev environment from the production one.
We would love this to be part of the main branch, this is really huge!

Hey, sorry for the radio silence, as you probably noticed the team efforts were shifted to other projects.
However, I agree this looks very useful, I will run the tests and try to merge it.

could you run npm run prettier-fix and add allowedOperations to a fixture in test/unit/fixtures/create-changeset-context-fixture.ts?
generally, PR looks really nice, I will try to add some tests after we merge it

could you run npm run prettier-fix and add allowedOperations to a fixture in test/unit/fixtures/create-changeset-context-fixture.ts?
generally, PR looks really nice, I will try to add some tests after we merge it

Thanks! I will do it

Hey @mayakarabula
Done and pushed :)

@EduSantosBrito
awesome, thank you :) will merge when CI passes

@EduSantosBrito
hey just fyi, I had to change and unify delimiter for queryEntries param, it now uses "=" everywhere
https://github.com/contentful/contentful-merge/pull/911


Move client consensus parts out of primitives and into client/consensus/api

This unlocks moving more client-only code further down the dependency tree.

Happy to bikeshead any name for BlockImportStatus except BlockImportResult - it's not a Result and calling it that just trips the reader up.

(please don't merge this without my review, which will come shortly)

Also needs a companion PR to polkadot, some imports there are broken now.

There is a companion PR: https://github.com/paritytech/polkadot/pull/3456 (but it's my first time so do please let me know if anything is out of kilter).

Sorry! Will stop rebasing and merge from master from now on.

bot merge


fix: extend validation schemas to allow dates across all timezones and all possible tag values

The current schemas for trust data validation neither allow timezones with negative offsets nor tags containing dashes. This fixes both and adds additional tests.
Fix #10

Codecov Report

Merging #15 into master will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master      #15   +/-   ##
=======================================
  Coverage   90.06%   90.06%
=======================================
  Files          14       14
  Lines         594      594
=======================================
  Hits          535      535
  Misses         59       59

Impacted Files
Coverage Δ

connaisseur/__main__.py
0.00% <0.00%> (ø)

Continue to review full report at Codecov.

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


adds a test-runner that sends notifications to all apps

This PR resolves this ticket: https://issues.jboss.org/browse/AGPUSH-2003
Adds a test-runner, written in nodeJS, to the current test suite. This test runner gets all applications from one user and send 1 notification to all of them with the single alias "TEST_TOKEN" that is the one mock-data-loader creates for all fake devices.
This test runner uses both community libraries: https://github.com/bucharest-gold/unifiedpush-admin-client (for retrieving the apps) and https://github.com/aerogear/aerogear-unifiedpush-nodejs-client/ (for sending the notifications).

Let's add a README file in the folder,
how to build/run this

OK I also noticed that the DELAY constant was not being used, so fixed


Add support for Policy Groups and Objects

It would be great if support would be added for Policy Groups and Objects (also sometimes refered to as Network Groups and Objects) which was introduced in API v1-11-0-beta-0.
https://documentation.meraki.com/MX/Firewall_and_Traffic_Shaping/Network_Objects_Configuration_Guide

This feature will be added to this library once it is GA.

Hi again. Policy objects are now GA as of 2022-11-02.

All supported in version 1.27.0.


Show name of the Evaluator in its progress bar (default="validation", which is the current behavior)

Evaluator with progress_bar=True shows this kind of simple progress bar which is very handy.
validation [#########################.........................] 50.00%
         1 iterations
    9642.1 iters/sec. Estimated time to finish: 0:00:00.000104.

The label of the progress bar is always set to "validation".
In case there are multiple evaluators set in a training loop, I feel that showing what is actually in progress right now would be slightly more helpful.
Each single Evaluator, which is an Extension, should basically have a unique name in an extension manager. Therefore I'd like to propose to show the name of an evaluator in its progress bar like below,
my_xxx_evaluator [#########################.........................] 50.00%
               1 iterations
          7358.4 iters/sec. Estimated time to finish: 0:00:00.000136.

in case the name of the Evaluator is set like this (as well as progress_bar=True).
evaluator = ppe.training.extensions.Evaluator(..., progress_bar=True)
evaluator.name = 'my_xxx_evaluator'

Length of the name is considered accordingly so that the progress bar layout is kept consistent.
(Even if you set a long/short name than default like the above example, 2nd and 3rd lines of the progress bar shift as necessary)
In typical use cases where we have only one evaluator and haven't explicitly set its name, it keeps the current behavior i.e., just shows "validation".

/test


Upgrade to Deno v0.42.0

I also added deno fmt --check to ci, but let me know if you want me to remove it.

LGTM


Use JSON source generator & enable trimming

This PR utilizes the JSON source generator and enables trimming. This should get us closer to enabling support for AOT builds.

@abergs @aseigler Ready for review.

Codecov Report

Merging #304 (a32d4de) into master (d25a42e) will increase coverage by 0.06%.
The diff coverage is 62.50%.

@@            Coverage Diff             @@
##           master     #304      +/-   ##
==========================================
+ Coverage   77.24%   77.30%   +0.06%
==========================================
  Files          86       87       +1
  Lines        2386     2388       +2
  Branches      401      401
==========================================
+ Hits         1843     1846       +3
+ Misses        442      441       -1
  Partials      101      101

Impacted Files
Coverage Δ

Src/Fido2.Models/AssertionOptions.cs
84.21% <0.00%> (ø)

Src/Fido2.Models/CredentialCreateOptions.cs
37.83% <0.00%> (ø)

...o2.Models/Objects/PublicKeyCredentialUserEntity.cs
0.00% <ø> (ø)

Src/Fido2/Metadata/FileSystemMetadataRepository.cs
0.00% <0.00%> (ø)

Src/Fido2/Metadata/MDSGetEndpointResponse.cs
100.00% <ø> (ø)

Src/Fido2/AuthenticatorResponse.cs
50.00% <100.00%> (ø)

Src/Fido2/Internal/GetBLOBRequest.cs
100.00% <100.00%> (ø)

...rc/Fido2/Metadata/ConformanceMetadataRepository.cs
89.91% <100.00%> (ø)

...c/Fido2/Metadata/Fido2MetadataServiceRepository.cs
60.19% <100.00%> (ø)

Continue to review full report at Codecov.

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


support globe-coordinate datatype

some notes:

[ ] two ways to match:

[ ] 1. two different parameters for longitude and latitude (similar to dates) having decimal values
[ ] 2. six different parameters where both longitude and latitude have three different parameters for degrees, minutes and seconds

[ ] optional globe?
[ ] auto precision?

This may be a little bit out of scope of HarvestTemplates :) , but we could let people add coords from geo_tags DB table or from API. That should be easier to implement, right?

datatype globe-coordinate is not yet supported brought me here :-)
My use case is importing templates such as the {{geo|33.9060845|133.7538068}} that can be seen at the bottom of the article https://en.wikivoyage.org/wiki/Oboke_and_Koboke
@edgarskos geo in my use case is a template, so it sounds very much on topic for HarvestTemplates.


Rebuild for Python 3.7, GCC 7, R 3.5.1, openBLAS 0.3.2

Checklist

[x] Used a fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[x] Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

@isuruf, @jakirkham, how can I rebuild this for the new compilers?

See https://github.com/conda-forge/numpy-feedstock/blob/master/conda-forge.yml#L3-L4

Thanks a lot @isuruf!

Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@conda-forge-admin, please rerender

Hi! This is the friendly automated conda-forge-webservice.
I tried to re-render for you, but it looks like there was nothing to do.

Thanks a lot @isuruf for your help!


ARROW-16430: [Python] Add support for reading record batch custom metadata API

In ARROW-16131, C++ APIs were added so that users can read/write record batch custom metadata for IPC file. In this PR, pyarrow APIs are added so that python users can take advantage of these APIs to address ARROW-16430.

By the way, this PR addresses RecordBatchFileReader but not RecordBatchReader. Do you plan to do that as well?

By the way, this PR addresses RecordBatchFileReader but not RecordBatchReader. Do you plan to do that as well?

I tried adding the pyarrow API for RecordBatchReader locally previously, but I found if I want to test the new pyarrow API for RecordBatchReader, I probably have to add the corresponding API implementation in C++ RecordBatchStreamReader so that I can unit test it in a meaningful way since it seems only RecordBatchStreamReader and RecordBatchFileReader provides the capability to read record batch custom metadata (is this correct?). And since this PR is more about adding new pyarrow API for existing C++ capability, I would like to make non necessary C++ API change less in this PR.

if there is other way to unit test pyarrow RecordBatchReader.read_next_batch_with_metadata API besides implementing it in RecordBatchStreamReader?
do you recommend to put all these changes together in single PR or keep them as separated PRs? I initially submitted this issue as a pyarrow enhancement PR, but I am okay to include more API changes if it is preferred.

@pitrou I made some additional changes to the PR, could you please help to review? Thanks.

@niyue Sorry for the late replay, but the C++ RecordBatchReader already has a method Result<RecordBatchWithMetadata> ReadNext(), so you wouldn't need to add anything more to the C++ side AFAICT?

@niyue Are you planning to work on this?
Otherwise @milesgranger you might be interested in taking this up?

@niyue Are you planning to work on this?
Otherwise @milesgranger you might be interested in taking this up?

Hi @pitrou, sorry I got too busy since last commit and forgot this issue previously. I am still interested working on this, and can start working on it from next week.

@niyue If/when this is ready for review, please say so :-)

@pitrou I pushed a new commit a minute ago. I followed your suggestion to add corresponding API for RecordBatchReader API in pyarrow, and a basic unit test to cover this API. I ran all pyarrow tests (using the minimum build only) and all tests passed locally. Could you please help to review? Thanks.

@pitrou I am not sure if you see my previous comment. I made some change previously, and this PR is ready for review, could you please help? Thanks.

@niyue Sorry, I had overlooked it. I'll take a look when I can. @jorisvandenbossche would you like to review this too?

No problem. I will check them out and see if I can get them addressed. Thanks for the review.

There were some timeouts in some CI jobs. I've restarted them just in case. If that still reproduces, should first rebase from master to catch up with any upstream fixes.

This looks like a nice addition. Something I am wondering more in general (potentially for future JIRA/PRs), when working with custom metadata, would it be useful to also allow to inspect the custom metadata of a certain batch, without also loading the batch? (so you could for example check the metadata before deciding whether to read the batch or not).
I suppose that would only make sense for the IPC file format? (although not sure if that makes sense at the format / C++ level) Like the RecordBatchFileReader now has a get_batch(i)/ get_batch_with_custom_metadata(i), one could also have a get_custom_metadata(i)?

Something I am wondering more in general (potentially for future JIRA/PRs), when working with custom metadata, would it be useful to also allow to inspect the custom metadata of a certain batch, without also loading the batch?

Perhaps, but that would have to be done on the C++ side first. Also, I would wait for users to actually request it.

@pitrou sorry for the late response. There are two failed CI builds, and they don't seem relevant in this issue. I rebased onto the latest master branch anyway.

@pitrou could you please help to review the PR to see if there is still anything we desire to change? Thanks.

No problem @pitrou . Thanks for the review and the fixes.


Parsing master material exports

Is your feature request related to a problem? Please describe.
Is it possible to parse these export types?
Material, MaterialExpressionMaterialFunctionCall, MaterialExpressionStaticSwithParameter, MaterialExpressionTextureSampleParameter2D...
M_COM_DetailMaster_B.zip
Describe the solution you'd like
Looking to be able to pull basic information from master materials in order to replicate them.

#45


SampleSet.from_samples_bqm should officially support BinaryPolynomials

SampleSet.from_samples_bqm accepts any object that has a .energies method and a vartype attribute instead of just BQMs, most notably BinaryPolynomial.
https://github.com/dwavesystems/dimod/blob/15e6f93638b937156e8f0fff008c3b63a451aa45/dimod/sampleset.py#L417
The docstring should mention this and tests should be added to check for backwards compatibility. I don't think the name of the method should be generalised, as this is just using python's duck typing.

This is also true of testing.assert_response_energies


implement travis build process after made public

As travis is free only for public repos

may as well do this now the repo is public

built successfully


libnet: 1.2-rc3 -> 1.2

Description of changes
cc @wkennington
Bump to tag v1.2
I am interested in this because it makes libnet provide a pkg-config file.
Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
22.05 Release Notes (or backporting 21.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[ ] Fits CONTRIBUTING.md.

Result of nixpkgs-review pr 166391 run on aarch64-darwin 1

8 packages built:

arping
arpoison
ettercap
libnet
netdiscover
ssldump
sslsplit
tcptraceroute


In Folders, when click the back button, the app exit.

Expected behavior
When click the Back button in 'Folders', want to return to Inbox, which is the previous page.
Actual behavior
When click the Back button in 'Folders', the app exit
Steps to reproduce

Move Inbox
Pressing the 'Folder' button in the upper right corner
In Folders, when press the back button, the app exit.

Environment
K-9 Mail version: 5.600
Android version: 8.0.0
Account type (IMAP, POP3, ##WebDAV/Exchange):

It's not really a back button. It's an 'up' button. If you only have one account there's nothing 'Up' from Folders.

"Up" (the button in the upper left corner) will always go to the Accounts activity. Pressing the device's back button will do different things depending on how you got to the screen. This doesn't make for a great user experience. But since we're changing the UI for 5.800 this will no longer be an issue.


Advanced settings to show more options

I don't know if this is even possible, but it would be great if we had an option that allows us to see more options. By default we we would see the simplest options on Settings to get a better and less confusing menu. I.e:

Accounts:

Manage existing accounts
Add new account
Manage microblogging systems (only if advanced settings are enabled)

Configuration:

Syncing
Storage (only if advanced settings are enabled)
Appearance
Notifications

Information:

About AndStatus
Changelog
Contact developer (only if advanced settings are enabled)
Logging and debugging
Advanced settings (NEW)

For example for Appearance settings:

Color theme
True black background (only if advanced settings are enabled)
Size
How to show a User in a Timeline (only if advanced settings are enabled)
Display Avatars
Display attached images
Display Microbblogng system in a Timeline (only if advanced settings are enabled)
ENTER key sends message (only if advanced settings are enabled)
Old messages first in a Conversation view
Mark replies to me in a Timeline
Alternative Notification icon (only if advanced settings are enabled)

I think this is doable. I like this approach, because it doesn't duplicate setting categories.
First, we will implement generic way to define any setting as Advanced or not, and then it will be easy to turn settings one by one...

Thanks, Yvolk!! ;)

I like this idea (it had occurred to me, too). That would also keep configuration simple for most people and still provides options for those who need more. UX improvement!


Add UWP support in WinUI3

Proposal: UWP Support
Summary
Let WinUI3 capable of building UWP apps
Rationale
1.We need WinUI3 to develop UWP apps for Xbox and Hololens
2.We need it to upgrade existing UWP apps
Important Notes
Support .Net Native to avoid reverse engineering
Open Questions

@EP012014 You can currently use WinUI3 as the UI layer on your native UWP apps!


Make a rule where to put a docstring for an init method

Some classes in Chainer put docstrings for __init__ methods into class docstrings. https://github.com/pfnet/chainer/blob/master/chainer/links/connection/linear.py
Some classes in Chainer put them into __init__'s own docstring. https://github.com/pfnet/chainer/blob/master/chainer/computational_graph.py
We should make a clear rule on which to prefer. Sphyinx's google style example says both are ok but should not be mixed. http://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html

I prefer the former one. And we can check if docstrings follow this rule with checking AClass.__init__.__doc__.


install broken with go 1.16 due to go modules

go modules are on by default with 1.16, and the repo seems to incorrectly declare its import path.  Trying go get results in an error:
vladimir@minim1 ~ % go get github.com/mwiora/NAMEinator
go: downloading github.com/mwiora/NAMEinator v0.0.5
go get<EMAIL_ADDRESS>updating to
<EMAIL_ADDRESS>parsing go.mod:
	module declares its path as: NAMEinator
	        but was required as: github.com/mwiora/NAMEinator

A fix for this would be really helpful because the new M1 MacBooks can only natively run go 1.16.x because they are on aarch64 processors. This means no-one with a newer MacBook would be able to build NAMEinator without installing an older version of go in an emulated x86_64mode through rosetta.

I join in with this.
Could have used the tool today, but it won't work on Manjaro.

Don't know hot to do this, I don't have any version-named directory in ~/go There's just 'pkg' inside. Could have used this tool today :(

Just run "go version" to find your version number (it'll be x.y.z, e.g. 1.17.2), then use mkdir to create the directory.

mkdir /Users/myusername/go/1.16.0/src/github.com/mwiora

add a  -p  to create intermediate directories...
mkdir -p  /Users/myusername/go/1.16.0/src/github.com/mwiora

Simply running
git clone https://github.com/mrwiora/NAMEinator.git
cd NAMEinator
go build

will work.


fix: update peer deps for React 19

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 80.53%. Comparing base (4bd2678) to head (747371a).

:exclamation:  There is a different number of reports uploaded between BASE (4bd2678) and HEAD (747371a). Click for more details.
HEAD has 3 uploads less than BASE

Flag
BASE (4bd2678)
HEAD (747371a)

4
1

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #308       +/-   ##
===========================================
- Coverage   91.04%   80.53%   -10.52%
===========================================
  Files          30       30
  Lines        2445     2445
  Branches      339      159      -180
===========================================
- Hits         2226     1969      -257
- Misses        219      470      +251
- Partials        0        6        +6

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


error: size of array '_check_for_64_bit_pointer_matching_GoInt' is negative

time ./make.bash fails with the following error:
runtime/cgo
In file included from $WORK/runtime/cgo/_obj/_cgo_export.c:2:0:
/data/local/tmp/go-build236511552/runtime/cgo/_obj/_cgo_export.h:35:14: error: size of array '_check_for_64_bit_pointer_matching_GoInt' is negative
typedef char _check_for_64_bit_pointer_matching_GoInt[sizeof(void*)==64/8 ? 1:-1];
Attempted fix:
export GOARCH=arm64
Full build log:
buildlog.txt
Similar issue:
https://github.com/golang/go/issues/10334
Platform: Kali Nethunter running on Nexus 9
Linux kali 3.10.40-g6857980-dirty #2 SMP PREEMPT Sat Nov 28 06:32:48 UTC 2015 aarch64 GNU/Linux

Have you solved your problem? I also met this:
** VERSION was not set, using default version. This is OK while in development.
Build tags: headless
Extra ldflags:
runtime/cgo
In file included from $WORK/runtime/cgo/_obj/_cgo_export.c:2:0:
/tmp/go-build661120796/runtime/cgo/_obj/_cgo_export.h:37:14: error: size of array ‘_check_for_64_bit_pointer_matching_GoInt’ is negative
typedef char _check_for_64_bit_pointer_matching_GoInt[sizeof(void*)==64/8 ? 1:-1];
^

Unfortunately, i wasn't able to fix the issue.


File System example error

Code provided raises the following syntax error:
const items = await fs.readdir("stores");
              ^^^^^

SyntaxError: await is only valid in async functions and the top level bodies of modules

Code:
const fs = require("fs").promises;

const items = await fs.readdir("stores");
console.log(items);

this is Top-level await, a new feature of ECMAScript 2022.

https://www.cnblogs.com/xgqfrms/p/16927849.html


Using with images having varying number of segments

I have a dataset of images containing varying number of segments. I also have pixel wise  labels for each of them. How can I use this dataset with this library?

If you have your own/custom dataset with ground truth (pixel-wise labels) available, it's just a matter of writing your own train and validation dataloaders. You should then be able to integrate it seamlessly with this repo.

Thanks. One quick question. Not all images have the same number of clusters. Will that cause any issue?

By clusters, I am assuming you mean number of classes present in an image? If so, then no. I would suggest looking into the datasets commonly used in semantic segmentation.

--dataset        Dataset to use ['pascal, camvid, ade20k etc']
In my case with custom datasets, is this parameter required while training?

Depends on how you implement it. If you are looking to do the same as done in this repo, then you need to modify your .json file to point to your dataset's location, and write the necessary loader scripts for train/validation routines. Look at __init__.py under loader.
Re code samples, I think there are quite a few repos on github where people have used these architectures on custom datasets.

I plan to add a base class for dataset in the update soon, so that you can inherit that class and create your own dataset iterator. Closing this for now?