"Status update" feedback

Hi there, a long time user of momentjs here.
I just read the status update and the recommendation to use other libraries. I just wanted to share my feedback that the status update seems a bit too harsh. At first look it seems very intimidating that this library has been declared as "legacy". Reading more carefully, if maintenance is indeed still happening as mentioned, I'm not sure there is a need in such a bold statement.
I think the library is well known by millions, the most popular one to date, referenced in numerous StackOverflow discussions, and well tested for many years. Its mutability is already well known by its users, so unless someone cares a lot about the bundle size - I don't see a real reason not to use it.
Personally, I would maybe include an advice to consider alternatives IF x and y, but would not declare this library as legacy.

Thanks for the feedback. If you are comfortable with the amount of maintenance Moment.js is getting, feel free to continue to use the library. We tried to explain what we mean by "legacy" in the post, since it has many meanings among developers. We put out this notice in part because:

We expect to put in fewer maintenance hours in the future.
This library may not update to support new JavaScript features and frameworks.
The strongly worded update gets attention of more Moment.js users out there, in case they would like to switch.

A central point of open-source software is that you can use this code if you like, but you cannot necessarily expect support. Quoting from the license: "Permission is hereby granted, free of charge, to any person..."


How to know when an image is already compressed or doesn't need to be compress ?

Hello,
Like the title said, is it possible to know when an image doesn't need to be compress ?
Because, if I reupload an already compressed file, I dont want it to be compressed again.
Other question, in my project, we let users upload their images in different formats and size, for you, what is the best settings ?
For now, I have this settings for the jpeg :
sharp().jpeg({ progressive:true, quality: 80, force: true })
Is this the best way ?
Thanks a lot for your time and effort in this module !

If the question here is "how do I know the 'quality' value used for a given JPEG?" then please see #908 for why this is not possible.

Hope this helped, please reopen with more details if not.


Added support for Text Input

Pull Request Etiquette

[x] I have checked the PRs for upcoming features/bug fixes.
[x] I have read the code of conduct.

Changes

[x] Internal code
[ ] Library interface (affecting end-user code)
[x] Documentation
[ ] Other: _____

Close : NaN
Description
Added support for Text Input means you can create things such as tickets easily.

Codecov Report
Base: 2.99% // Head: 2.97% // Decreases project coverage by -0.02% :warning:

Coverage data is based on head (2b360ff) compared to base (eaeaf2a).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##             master    #118      +/-   ##
===========================================
- Coverage      2.99%   2.97%   -0.03%
  Complexity       61      61
===========================================
  Files           377     380       +3
  Lines          6767    6826      +59
  Branches        653     660       +7
===========================================
  Hits            203     203
- Misses         6533    6592      +59
  Partials         31      31

Impacted Files
Coverage Δ

...ent/events/interaction/textinput/TextInputEvent.kt
0.00% <0.00%> (ø)

...r/handlers/interactions/MessageComponentHandler.kt
0.00% <0.00%> (ø)

...mpl/interaction/message/textinput/TextInputImpl.kt
0.00% <0.00%> (ø)

...wk/ydwk/interaction/message/textinput/TextInput.kt
0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


[HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…

…dieTableConfig
What is the purpose of the pull request
Currently we use HoodieTableMetaClient#initTableType to init the table properties. As the number of Table Properties increases，It is hard to maintain the code for the initTableType method, especially when we add a new table property. Support Builder Pattern to build the table config properties can solve this problem.
Brief change log

Add PropertyBuilder to build the table properties and init the table.
Remove the origin initTableType method in HoodieMetaClient.
Refactor all the code refer to initTableType

Verify this pull request
(Please pick either of the following options)
This pull request is a trivial rework / code cleanup without any test coverage.
(or)
This pull request is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)

Added integration tests for end-to-end.
Added HoodieClientWriteTest to verify the change.
Manually verified the change by running a job locally.

Committer checklist

[ ] Has a corresponding JIRA in PR title & commit

[ ] Commit message is descriptive of the change

[ ] CI is green

[ ] Necessary doc changes done or have another open PR

[ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

Codecov Report

Merging #2596 (b79eb00) into master (43a0776) will decrease coverage by 1.76%.
The diff coverage is 63.09%.

@@             Coverage Diff              @@
##             master    #2596      +/-   ##
============================================
- Coverage     51.14%   49.38%   -1.77%
+ Complexity     3215     2846     -369
============================================
  Files           438      381      -57
  Lines         20041    17145    -2896
  Branches       2064     1735     -329
============================================
- Hits          10250     8467    -1783
+ Misses         8946     8008     -938
+ Partials        845      670     -175

Flag
Coverage Δ
Complexity Δ

hudicli
37.01% <100.00%> (+0.14%)
0.00 <0.00> (ø)

hudiclient
100.00% <ø> (ø)
0.00 <ø> (ø)

hudicommon
51.40% <48.93%> (+0.03%)
0.00 <0.00> (ø)

hudiflink
46.46% <100.00%> (+1.02%)
0.00 <0.00> (ø)

hudihadoopmr
33.16% <ø> (ø)
0.00 <ø> (ø)

hudisparkdatasource
?
?

hudisync
?
?

huditimelineservice
?
?

hudiutilities
69.47% <69.56%> (+<0.01%)
0.00 <0.00> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ
Complexity Δ

...pache/hudi/common/table/HoodieTableMetaClient.java
77.38% <ø> (+6.61%)
42.00 <0.00> (-2.00)
:arrow_up:

...rg/apache/hudi/common/table/HoodieTableConfig.java
46.77% <48.93%> (+1.31%)
17.00 <0.00> (ø)

...apache/hudi/utilities/deltastreamer/DeltaSync.java
69.31% <50.00%> (-0.69%)
50.00 <0.00> (ø)

...ava/org/apache/hudi/cli/commands/TableCommand.java
87.80% <100.00%> (+1.69%)
7.00 <0.00> (ø)

...c/main/java/org/apache/hudi/util/StreamerUtil.java
41.50% <100.00%> (+1.12%)
12.00 <0.00> (ø)

...udi/utilities/deltastreamer/BootstrapExecutor.java
82.35% <100.00%> (+2.80%)
6.00 <0.00> (ø)

...udi/operator/partitioner/BucketAssignFunction.java
79.54% <0.00%> (-12.77%)
18.00% <0.00%> (+10.00%)
:arrow_down:

...ain/java/org/apache/hudi/avro/HoodieAvroUtils.java
55.66% <0.00%> (-0.44%)
38.00% <0.00%> (ø%)

...org/apache/hudi/HoodieDatasetBulkInsertHelper.java

...rc/main/scala/org/apache/hudi/Spark3RowSerDe.scala

... and 55 more


[E2B-457] Unable to create custom templates with Dockerfiles containing more than ~4.3GB of local data

When a user is creating a custom template, they specify a Dockerfile. If the Dockerfile requires to copy more than 4,294,967,295 bytes of a local data, it will fail with the following message
Preparing sandbox template building (35639 files in Docker build context).
Found ./e2b.Dockerfile that will be used to build the sandbox template.
node:internal/blob:165
      throw new ERR_BUFFER_TOO_LARGE(0xFFFFFFFF);
      ^
RangeError [ERR_BUFFER_TOO_LARGE]: Cannot create a Buffer larger than<PHONE_NUMBER> bytes
    at new NodeError (node:internal/errors:399:5)
    at new Blob (node:internal/blob:165:13)
    at R1.<anonymous> (/Users/default/Library/pnpm/global/5/.pnpm/@e2b+cli@0.1.20_openai@4.24.1/node_modules/@e2b/cli/dist/index.js:104:1093)
    at R1.emit (node:events:513:28)
    at n1.updateNonPrimary (/Users/default/Library/pnpm/global/5/.pnpm/@e2b+cli@0.1.20_openai@4.24.1/node_modules/@e2b/cli/dist/index.js:54:11372)
    at n1.update (/Users/default/Library/pnpm/global/5/.pnpm/@e2b+cli@0.1.20_openai@4.24.1/node_modules/@e2b/cli/dist/index.js:54:11193)
    at n1.ez (/Users/default/Library/pnpm/global/5/.pnpm/@e2b+cli@0.1.20_openai@4.24.1/node_modules/@e2b/cli/dist/index.js:54:13685)
    at process.processTicksAndRejections (node:internal/process/task_queues:77:11) {
  code: 'ERR_BUFFER_TOO_LARGE'
}
Node.js v18.16.1

We're currently using Node's Blob to represent the files and upload them all at once. One of the potential solutions to this issue is to start streaming the data in chunks to the backend.
From SyncLinear.com | E2B-457

The maximum sizes of a Buffer in Node is described here.

On 32-bit architectures, this value currently is 230 - 1 (about 1 GiB).
On 64-bit architectures, this value currently is 232 (about 4 GiB).

Solved by fc037a2b


Feature request: Option to display column letters

Like in Excel or Google Sheets where it displays letters for each column at the top. Often times my coworkers send me spreadsheets (which I always convert to csv) and ask me things like "look at column Z for this and colum Y for that" and since I don't have Excel I have to upload it to Google Docs just to see which column they're talking about.
So it would be nice to have some option to display column letters above the column headers.

This is interesting and should be possible. Maybe something like a --spreadsheet option.


fix asynchronous method in bitflyer

'await' is necessary when using asynchronous processing in bitflyer
I fixed because of getting the following error

bitflyer = ccxt.bitflyer({})
loop = asyncio.get_event_loop()
a = loop.run_until_complete(bitflyer.fetchTicker('FX_BTC_JPY'))
print(a)
loop.close()

Traceback (most recent call last):
  File "asynctest.py", line 24, in <module>
    a = loop.run_until_complete(bitflyer.fetchTicker('FX_BTC_JPY'))
  File "/Users/taish/anaconda3/lib/python3.6/asyncio/base_events.py", line 466, in run_until_complete
    return future.result()
  File "/Users/taish/anaconda3/lib/python3.6/site-packages/ccxt/async/exchange.py", line 181, in fetchTicker
    return await self.fetch_ticker(symbol)
  File "/Users/taish/anaconda3/lib/python3.6/site-packages/ccxt/async/exchanges.py", line 2301, in fetch_ticker
    await self.load_markets()
  File "/Users/taish/anaconda3/lib/python3.6/site-packages/ccxt/async/exchange.py", line 143, in load_markets
    markets = await self.fetch_markets()
  File "/Users/taish/anaconda3/lib/python3.6/site-packages/ccxt/async/exchanges.py", line 2253, in fetch_markets
    for p in range(0, len(markets)):
TypeError: object of type 'coroutine' has no len()

Hi! Good catch, thank you so much for this notice! And for the PR fix as well. Your contributions are very helpful, thanks for your effort. Merging it now!

But... I think the problem there is not the missing await (because it's missing in 16 different other places and it is actually missing intentionally – should not be an await there, the await should be outside most of the time, when we don't preparse the reply).
The actual cause of the problem is using the 'FX_BTC_JPY' symbol. It is present in the list of symbols, but there's no such market in the list of markets for bitflyer for some reason. I am debugging it now to resolve this issue. Thx for your help once again!

thanks!

Ok, @xpl helped me to find this bug, and the problem was the async decorator with the bitlish.request(...) method. We have uploaded the fix for it and it should be working normally in a new version, which will be available in about 20-30 minutes.

This is fully fixed as of version 1.7.46+. Thx again for reporting!


feat(starknet-accounts): Estimate Fee for Account

Exposes a fee estimation function from within the starknet-accounts/src/account.rs Account trait.
This follows starknet.js' convention defined here.

Hey thanks for the PR but the CI is failing. Could you please resolve?

Hey thanks for the PR but the CI is failing. Could you please resolve?

Fixed :)

:point_right::point_left: Could y'all merge this?

Sorry for not reviewing this PR in time, but since this very same functionality has been implemented in #115, I'm closing this one here.


Crash when leaving in recents on detail

Crashes if you leave in detail activity and then come back and try to go back and select another card

Was not able to reproduce


examples/dmabuf-capture: use getopt

This makes it a little bit less annoying to provide the right arguments.
All options have reasonable defaults.

Thanks!


expr/builtin_fncall: implement MultiplyIntUnsigned

What have you changed? (mandatory)
This PR implements the scalar function MultiplyIntUnsigned
What are the type of the changes? (mandatory)
The currently defined types are listed below, please pick one of the types for this PR by removing the others:

New feature (non-breaking change which adds functionality)

How has this PR been tested? (mandatory)
unit test
Does this PR affect documentation (docs/docs-cn) update? (mandatory)
no
Does this PR affect tidb-ansible update? (mandatory)
no
Refer to a related PR or issue link (optional)
https://github.com/pingcap/tikv/issues/3275
The related source in tidb: https://github.com/pingcap/tidb/blob/master/expression/builtin_arithmetic.go#L532
@zz-jason @XuHuaiyu @breeswish PTAL

LGTM! BTW I saw "MultiplyIntUnsigned" occurred in multiple places. Since we are already using macros, can we just eliminate it to one place to be more friendly to the contributor?

Would you pull the request for it @breeswish ?

@AndreMouche Love to!

/run-integration-tests


Numerical summaries of edge bearing distributions

Hello, thanks again for this great package. I'm really interested in OSMnx's ability to derive street edge bearings. I'd like to use that property of street networks in a regression and so need a numerical summary of bearing distribution for a graph. I see from this article that a normalised measure of orientation order (φ in the article) can be calculated. However, I was wondering whether osmnx already had this feature.

It's not built in, but you can adapt the code from here: https://github.com/gboeing/osmnx-examples/blob/master/notebooks/17-street-network-orientations.ipynb
Once you get the distribution of bearings using that code, you can just apply the formulas to calculate the entropy then convert it to orientation order as described in the article.


Add timeouts options for traefik

What this PR does / why we need it:
Add Traefik Responding Timeouts configuration options in traefik chart.
Which issue this PR fixes
No issues related.
Special notes for your reviewer:
Checklist

[x] DCO signed
[x] Chart Version bumped
[x] Variables are documented in the README.md

@seuf Can you modify this PR so that those values are not mandatory? You will have to check if they exist before use.
Thanks!

@seuf Can you modify this PR so that those values are not mandatory? You will have to check if they exist before use.
Theses values are already not mandatory. There is already a check if variables are not empty before using it in configMap. Do you want I remove the defaults values (which are also the default of Traefik)
I've rebased over master to fix conflict.


user guide's page 241- Unmapped Implementation error

Line 8 of the unmapped implementation in the user guide's page 241 has this line:
vector[2] beta[K];
And this produces the following error:
SYNTAX ERROR, MESSAGE(S) FROM PARSER:

No matches for:

  real[] .* vector

Available argument signatures for operator.*:

  vector .* vector
  row vector .* row vector
  matrix .* matrix

No matches for:

  real[] + ill formed

Available argument signatures for operator+:

  int + int
  real + real
  vector + vector
  row vector + row vector
  matrix + matrix
  vector + real
  row vector + real
  matrix + real
  real + vector
  real + row vector
  real + matrix
  +int
  +real
  +vector
  +row vector
  +matrix

Expression is ill formed
  error in 'model1a7a481d4cfe_no_map_rect' at line 18, column 53
  -------------------------------------------------
    16:   for (i in 1:2)
    17:     beta[ , i] ~ normal(mu[i], sigma[i]);
    18:   y ~ bernoulli_logit(beta[kk, 1] + beta[kk, 2] .* x);
                                                            ^
    19: }
  -------------------------------------------------

I'm pretty sure that this line should be matrix[K, 2] beta;
which fixes the error.
Current Version:
v2.18.0

This issue was moved to stan-dev/docs#1

Thanks for reporting.  I moved the issue because the source is now in the repo docs.


StagedWelcome::new_from_welcome_body for processing Welcome messages

Closes #1519
This PR provides an additional method within StagedWelcome for creating a new group from a Welcome message.  This is in addition to the current API, which provides StagedWelcome::new_from_welcome for creating a new group from a MlsMessageIn.

Closing based on the following guidance in #1519:

We should not have two ways to process Welcome messages.


Set up Github Codespaces devcontainer and startup bash scripts

I've been using Github Codespaces for quick set up. Will be adding config files for Codespaces for this repo.

Sent in a pull request for this :) My first pull request on Github ever! hahaha


Publish to arbitrary git repo?

One feature that would be helpful is the ability to publish to a non-Github git repo.
This is an outstanding product, just what I was looking for in an editor.

I too hope that this feature will be integrated soon.

:+1:

I know, this thread is kinda old, but with the availability of packages like https://www.npmjs.com/package/simple-git would you consider enabling of access to arbitrary git repros?

I believe the maintainer closed this issue a few years back. One side-note: I think simple-git is probably not as good a choice  for this as something like nodegit (https://www.npmjs.com/package/nodegit), which does not depend on running external executables in the shell environment. The real problem may have to do with authentication- github provides an api for interacting directly with the git repo, authenticated with OAuth, which means StackEdit does not need to store the keys to your git repo...


Allow creation of invoices with msat precision

Hi!
These changes allow create invoices with msat precision.
No gui changes, except allow wallet balance show fractional part of sats.
Intended for use via API calls.

Looking why regtest fails.

rebased on current main

Given the many uncertainties even in sats denominated invoices (withholding thresholds, channel close reservations, etc, etc.) what is the benefit in permitting user specification of amounts as, currently admittedly, insignificant as millisats?
LNbits is not designed for either very large, or microscopically tiny, amounts.  It seems likely this will cause more trouble than it's worth.


--text  should not differiate between rfc and I-D rendering

--text outputs formatted text to file, with proper page breaks
this is only done when rendering an I-D, not an RFC - which is a weird distinction, either forgo page breaks entirely (the new default) or unify the behavior.

The disadvantage of this consistency is that page breaks are really useful for collaborating on documents, so they should be on for I-Ds, and they are not desired in RFCs according to 799x.  So the default actually makes a lot of sense.

disadvantage -> advantage?
This might be a broader discussion, but if page-breaks on not in the final output they should also not be in intermediate ones. "See Page 5" can be perfectly fine in I-D stage, but not later. To me that's rather strange.
Another point may be that not having pages in those early stages brings in to focus on how we refer to things, by para number or other things, or line numbers (neither of which are currently rendered in the output(s)) - except when you have an index

The RFC editor will make sure there is no "see page 5" (or "see section 2" for that matter).
Currently, we have no way to point into a longer piece of FDT (YANG, CDDL, ...), so page numbers are needed to do that.  I wouldn't want to cripple that.

We made a deliberate decision to paginate drafts but not RFCs.
I find page breaks useless for collaboration, because I look at the HTML version on my tablet. If people have suggestions for ways to put better anchors in long sections, those could be interesting.

We definitely need a way to put anchor points into source code and similar display artwork.
(A similar kind of in-text anchoring is also needed for math, for equation numbers.)
This will need significant work if the content of the code blocks is intended to remain extractable.
Once we get back to a state where we can improve RFCXML again (as opposed to waiting for simple bugfixes to occur), we should tackle that.
That of course only works where the author intended to have in-block anchors.  Rendering simple line numbers could provide a more general "phone-surviving" way of pointing into code blocks.  (These of course will eat further into the 69-character limit.)

then the help text for --text should probably mention this


[WIP] ci: Remove unused submodules config

this also allows the the checkout action to be updated
Signed-off-by: Ryan Northey<EMAIL_ADDRESS>
Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

/wait for #24212 to ensure changes are properly tested

/retest


Problems reading 16-bit PNG?

I'm trying to read a 16-bit PNG file and print out the RGB values in the first pixel. They consistently differ from the output of Matlab or an image viewer. I cannot figure out why... If I open the image in Rust, then save it to a new file, and then open that file in Matlab or an image viewer, the resulting values are as expected, so I am not sure what is going on here.
For example, Rust gives the following:
RGB { r: 20877, g: 42635, b: 33940 }
whereas Matlab gives this:
[36177, 35750, 38020]
Matlab is also correct, whereas Rust is wrong.
I ran this Rust code:
extern crate lodepng;

use std::path::Path;

fn main() {
    let fin = std::env::args().nth(1).unwrap();
    let img = lodepng::decode_file(&Path::new(&fin), lodepng::LCT_RGB, 16).unwrap();

    let bitmap = match img {
        lodepng::Image::RGB16(ref bitmap) => bitmap,
        _ => panic!("??"),
    };
    let (w, h) = (bitmap.width, bitmap.height);
    let bitmapbuf = bitmap.buffer.as_ref();

    let p = bitmapbuf.iter().nth(0).unwrap();

    println!("{:?}", p);
}

Best,
Rob

This is because lodepng doesn't do endian swapping. You'll need byteorder crate to read the numbers.

I figured as much and tried using byteorder, but I guess I was confused. Should I be processing them as Little Endian then? I tried Big Endian out of habit, but I should have thought to test the other. Will do that now. Thanks!

Ok, it looks to be working correctly now. Thanks a lot! For future reference, the working code:
extern crate lodepng;
extern crate byteorder;

use std::path::Path;
use std::io::Cursor;

use byteorder::{WriteBytesExt, ReadBytesExt, BigEndian, LittleEndian};

fn main() {
    let fin = std::env::args().nth(1).unwrap();
    let img = lodepng::decode_file(&Path::new(&fin), lodepng::LCT_RGB, 16).unwrap();

    let bitmap = match img {
        lodepng::Image::RGB16(ref bitmap) => bitmap,
        _ => panic!("??"),
    };
    let (w, h) = (bitmap.width, bitmap.height);
    let bitmapbuf = bitmap.buffer.as_ref();

    let p = bitmapbuf.iter().nth(0).unwrap();

    // convert to proper endian-ness for processing
    let mut wtr = vec![];
    wtr.write_u16::<BigEndian>(p.r).unwrap(); // write R channel - encoded in Big Endian
    wtr.write_u16::<BigEndian>(p.g).unwrap(); // write G channel - encoded in Big Endian
    wtr.write_u16::<BigEndian>(p.b).unwrap(); // write B channel - encoded in Big Endian

    // wrap in a std::io::Cursor to use byteorder reading functions
    let mut rdr = Cursor::new(wtr);
    let R = rdr.read_u16::<LittleEndian>().unwrap(); // read R channel - Little Endian
    let G = rdr.read_u16::<LittleEndian>().unwrap(); // read G channel - Little Endian
    let B = rdr.read_u16::<LittleEndian>().unwrap(); // read B channel - Little Endian

    println!("{:?}", (R, G, B));
}

Best,
Rob


[SR-426][Runtime] Use _fail to ensure src is deallocated in cast

This case was previously ignoring the DestroyOnFailure flag, so
we had a leak if a cast to an existential metatype failed for
certain types (tuples, structs, etc).

+1, thanks for doing this.

I see two similar calls to swift_dynamicCastFailure() inside _dynamicCastMetatypeToExistentialMetatype(). Do you think they have the same problem?

Yes, good point. I'll fix it there, too, though I'm not sure how to write an analogous test for that.

OK. I've looked more into this and decided that one of the callers to _dynamicCastMetatypeToExistentialMetatype appears to expect it to not deallocate its passed src, as it's not passing the original srcValue, but the logic is hard for me to follow. It also looks to me like there are other similar bugs, like _dynamicCastToExistentialMetatype calling itself with a different srcValue but without changing the memory managemnt flags here, which (unless there's some invariant I don't understand) means the original PR might not be safe since it could now deallocate that new value.
I think I might try a local refactor to introduce some types (e.g. have _fail and _succeed be members on a struct type which is threaded through so we can be sure that they're called exactly once) or otherwise try to centralize the memory-management behavior here. Does that sound reasonable, or would you rather I just try to fix the issues I've identified?

Casting.cpp needs an overhaul. It's tangled and insufficiently tested. However that should not hold up immediate fixes like this.
Is this bug caused by commit e09027c, or is that unrelated?

It's unrelated; that commit is the cause the regression my other PR fixes but not this one.

Okay, that means "back out e09027c" is not the right answer here. Let's take this fix now and wrangle with Casting.cpp separately.

OK, so then we still might want to worry about the fact that _dynamicCastToExistentialMetatype is not removing the memory management flags here because as it stands this PR could cause an overrelease in that code path. I can add that in tomorrow.

OK. I tried to reproduce the overrelease I was expecting to be there, but couldn't. After reviewing the code and value-witness docs more closely, I believe the PR is correct as is. It is now essentially the same logic as _dynamicCastFromExistential which can ultimately call _fail on the underlying value and also deallocate the opaque existential buffer via _maybeDeallocateOpaqueExistential.
My confusion stemmed from the fact that _maybeDeallocateOpaqueExistential doesn't actually touch the underlying value at all, just the buffer, so freeing the value separately makes sense.
So, I think this ready to be merged.

Done. I still don't trust this file. It needs comprehensive tests.


Attribute error in gallery action

Action is failing with an attribute error - see https://github.com/ProjectPythia/cookbook-gallery/issues/165
Run python .github/workflows/collect-user-submission.py
Traceback (most recent call last):
  File ".github/workflows/collect-user-submission.py", line 38, in <module>
    issue = IssueInfo(gh_event_path=os.environ["GITHUB_EVENT_PATH"]).create_submission()
  File ".github/workflows/collect-user-submission.py", line 23, in create_submission
    self._create_submission_input()
  File ".github/workflows/collect-user-submission.py", line 27, in _create_submission_input
    text = self.data["issue"]["body"]
AttributeError: 'IssueInfo' object has no attribute 'data'
Error: Process completed with exit code 1.

This is an attempt to update the .py script to fix this. Related to #158

I don't understand it much either, but since it's already broken, I don't see a good reason not to ahead and merge for further testing @jukent

Thank you. I'm not 100% confident but it it is a possible workaround if self.data doesn't exist. Let's merge and see.


Undo the KeyVaultKeyTests.java deletion

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


rsa: Add BN_free() for BN_secure_new()

Add the BN_free() in the error handler to release the "rsa->p" and "rsa->q" allocated by BN_secure_new(), like the one in ossl_rsa_sp800_56b_derive_params_from_pq().
Fixes: 8240d5fa65 ("FIPS 186-4 RSA Generation & Validation")

Checklist

[ ] documentation is added or updated
[ ] tests are added or updated

You would be better off removing...
    if (rsa->p == NULL)
        rsa->p = BN_secure_new();
    if (rsa->q == NULL)
        rsa->q = BN_secure_new();

and leaving in the check that p and q are not NULL on entry to this function.

You would be better off removing...
    if (rsa->p == NULL)
        rsa->p = BN_secure_new();
    if (rsa->q == NULL)
        rsa->q = BN_secure_new();

and leaving in the check that p and q are not NULL on entry to this function.

Yes. Even though it is not a bug, I think it is a good programming mode that release the allocated momory in the same function when error occurs. But anyway, it is not a bug.

closing a pull request for now, code will be left-as-is.


Don't accept unmatched string delimiters in HLZASM

Now "Foo' isn't a valid string constant, but "isn't" is.
Fixes #1011

@suunrider would you be able to test this? I won't have a computer at my house until Saturday.

The string "Foo' errors with Newline in string constant, also the editor highlights it like a normal string.

@suunrider okay, I've changed the error message for "Foo' and the editor should highlight it in red. (There seems to be no proper syntax error highlighting colour?)

There aren't any syntax checks in the zcpu editor, so doesn't "notfound" color exists. Just add ["notfound"] = { Color(240, 96, 96), false}, to the colors (copied from e2).
The error messages work properly now :+1: .

Whoops, I didn't realise the two syntax highlighters had their own syntax colour tables. Fixed.

The error highlighting ain't working: "Foo gets highlighted as string

Okay, I'll wait until I have a computer I can run and test my code on. Thanks for your help.

In most editors I've used "foo would be highlighted as a string. (I don't know the reason - maybe so they don't change colour when you finish typing them)

If it's meant to be like C then 'A' should be the number 65 (ASCII code of A) rather than a string.

It already does that (see hc_tokenizer.lua#L288).

Why did you change the C-Style operators to lua ones?

Because my editor doesn't understand them, and those particular ones were causing it to show errors down the entire rest of the file, and we have a rule that the C-style operators shouldn't be used in our code. So I figured it'd be better to just sneak in that unrelated improvement to the codebase while I'm there :wink:

Oh, some codestyle guidelines would be useful then since there are many different coding styles in wiremod

#350 was the closest we got, but you're right that it would be worth having some documentation somewhere.

Once ported the HL-ZASM Compiler out of gmod: https://github.com/suunrider/hlzasm. Uploaded it to github, i thought it maybe would be useful for you. I don't have a zcpu emulator yet, so it's only useful to check if something parses.


replace FlatList with .map solution

Explanation of Change

FlatList was replaced with a data.map(() => {}) solution to get rid of the flashing effect.
Fixed Issues

$ https://github.com/Expensify/App/issues/53027
PROPOSAL: https://github.com/Expensify/App/issues/53027#issuecomment-2520578424
Tests

Open the web app and log into your account
Enable debug mode in Troubleshoot
Open an expense thread (create one if you don't have any)
Enter debug view then check on "View transaction" tab
Go to "violations" > Create a violation and save it
You might need to repeat step 5 many times in a row to be able to notice the flashing effect

[x] Verify that no errors appear in the JS console

Offline tests

N/A
QA Steps

Same as Tests section above

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

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

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I used JaimeGPT to get English > Spanish translation. I then posted it in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.ts or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

MacOS: Chrome / Safari
https://github.com/user-attachments/assets/34b4276f-5042-4578-a10f-943e8780c1b1

@jacobkim9881 Let's do the same thing on DebugReportActions

@DylanDylann Could please you explain what do you want to me?

@DylanDylann done!

Sorry, @jacobkim9881 I want to tag @JKobrynski

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

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

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

@jacobkim9881 I still get flicker
https://github.com/user-attachments/assets/74332109-db86-4ed0-895a-d547ffdf73f6

@JKobrynski From my investigation, the bug comes from
https://github.com/Expensify/App/blob/0ef4fb17c867b50f8495cda5127c8f0c537e9c35/src/pages/Debug/TransactionViolation/DebugTransactionViolationCreatePage.tsx#L86-L87
We update Onyx data and then navigate to a new page, but the navigation action is executed before completely updating ONYX. This is a common way to address this bug in other places
        Debug.setDebugData(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`, [...(transactionViolations ?? []), parsedTransactionViolation]);

        Navigation.setNavigationActionToMicrotaskQueue(() =>
            Navigation.navigate(ROUTES.DEBUG_TRANSACTION_TAB_VIOLATIONS.getRoute(transactionID)),
        );

@DylanDylann all right, I will try that solution. Thanks!

@DylanDylann I'm back from my vacation, so I'll take it from here 😄

@DylanDylann I tried your fix but the issue still happens. I found a way to easily reproduce the issue by throttling the CPU on Google Chrome.
Tomorrow, I'll investigate it further 😄

@DylanDylann
Last friday (12/13/24)

I did some trial and error to understand what could be causing OnyxTabNavigator to re-render the page contents, but I didn't manage to find the cause.

Today:

I created my own tab navigator, DebugTabNavigator, which mimics OnyxTabNavigator behaviour and layout but uses a StackNavigator instead of a MaterialTopTabNavigator and it fixes the problem.

@DylanDylann sorry for the delay here, yesterday I thought I had addressed all eslint issues 😅
It's ready now for your review 😄

Thanks

Update: I will try to complete the checklist on Friday

@DylanDylann all conflicts resolved 😄

@pac-guerreiro Thanks, this is a great refactor

Reviewer Checklist

[x] I have verified the author checklist is complete (all boxes are checked off).
[x] I verified the correct issue is linked in the ### Fixed Issues section above
[x] I verified testing steps are clear and they cover the changes made in this PR

[x] I verified the steps for local testing are in the Tests section
[x] I verified the steps for Staging and/or Production testing are in the QA steps section
[x] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I checked that screenshots or videos are included for tests on all platforms
[x] I included screenshots or videos for tests on all platforms
[x] I verified tests pass on all platforms & I tested again on:

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

[x] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[x] I verified proper code patterns were followed (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I verified that this PR follows the guidelines as stated in the Review Guidelines
[x] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:

[x] I verified that all the inputs inside a form are aligned with each other.
[x] I added Design label and/or tagged @Expensify/design so the design team can review the changes.

[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native

Android: mWeb Chrome
https://github.com/user-attachments/assets/f7187517-a690-4804-8bc1-e102ba5f0c68

iOS: Native

iOS: mWeb Safari

https://github.com/user-attachments/assets/673230b7-bc82-477c-be85-c7d7a2d5c9ff

MacOS: Chrome / Safari
https://github.com/user-attachments/assets/669cca9a-3e3a-4752-a7d8-124fca3fea7a

MacOS: Desktop

https://github.com/user-attachments/assets/16de9e0a-149a-43c8-864b-8b51a3fc2827

@pac-guerreiro App crash on native
https://github.com/user-attachments/assets/7ebb083e-a835-4942-b549-482392100d5b

@DylanDylann I fixed the crash, thanks! 😄

@DylanDylann
I'll be away until January 2nd but someone should take care of my work 😄 Happy holidays! 🎉

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

Hmm, I don't think this should be considered a deploy blocker since this should only be affecting internal debugging tools. We intentionally used a less performant method of rendering a list so that we could fix a bug.

I need to be afk for the rest of the day, but I will check in on this tomorrow morning my time (PST). If we still think it might be a legit issue then we should probably just revert. For now I'll give time for other people to come online and take a look at this.

Removing the deploy blocker label because after running the performance tests again the linking actually took less time than the previous run.
1st performance regression run:
566.861 ms → 669.112 ms (+102.251 ms, +18.0%)

2nd performance regression run:
540.412 ms → 539.824 ms (-0.588 ms, ±0.0%)

Conversation ongoing here, but I think its safe to not block the deploy on this.


Fixed "SharePoint Look Book" broken link

Category

[x] Content fix

Related issues

fixes #9862

What's in this Pull Request?
Fixed "SharePoint Look Book" broken link

Thx @nanddeepn - important to fix the broken links 👍
@andrewconnell - FYI - claimed this from you (PR was assigned to you) as was working on the clean up this morning - so got this also now processed.


issue #28 Rename to extensions

Signed-off-by: mloufra<EMAIL_ADDRESS>Description
Rename method names, class names, variable names from plugin to extensions in the sdk codebase and related OpenSearch code
Equivalent PR on SDK: https://github.com/opensearch-project/opensearch-sdk/pull/72
Issues Resolved
https://github.com/opensearch-project/opensearch-sdk/issues/28
Check List

[ ] New functionality includes testing.

[ ] All tests pass

[ ] New functionality has been documented.

[ ] New functionality has javadoc added

[ ] Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Codecov Report

Merging #4063 (a120dd2) into feature/extensions (716756a) will decrease coverage by 0.09%.
The diff coverage is 72.96%.

:exclamation: Current head a120dd2 differs from pull request most recent head 8a263eb. Consider uploading reports for the commit 8a263eb to get more accurate results

@@                   Coverage Diff                    @@
##             feature/extensions    #4063      +/-   ##
========================================================
- Coverage                 70.64%   70.55%   -0.10%
+ Complexity                57129    57058      -71
========================================================
  Files                      4612     4603       -9
  Lines                    274811   274553     -258
  Branches                  40227    40211      -16
========================================================
- Hits                     194142   193708     -434
- Misses                    64407    64601     +194
+ Partials                  16262    16244      -18

Impacted Files
Coverage Δ

...nsearch/gradle/testclusters/OpenSearchCluster.java
0.00% <0.00%> (ø)

...opensearch/gradle/testclusters/OpenSearchNode.java
0.00% <0.00%> (ø)

...lustermanager/info/TransportClusterInfoAction.java
61.53% <ø> (ø)

.../org/opensearch/client/support/AbstractClient.java
34.60% <0.00%> (-0.18%)
:arrow_down:

.../java/org/opensearch/cluster/ack/AckedRequest.java
0.00% <0.00%> (ø)

...rg/opensearch/common/settings/ClusterSettings.java
91.89% <ø> (-0.22%)
:arrow_down:

...src/main/java/org/opensearch/test/TestCluster.java
60.00% <ø> (ø)

...ore/restore/TransportRestoreRemoteStoreAction.java
25.00% <25.00%> (ø)

...opensearch/search/internal/ShardSearchRequest.java
79.91% <28.57%> (-1.75%)
:arrow_down:

.../replication/checkpoint/ReplicationCheckpoint.java
53.48% <33.33%> (-0.18%)
:arrow_down:

... and 566 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.


TextField Box component doesn't account for alignment

It appears that the worldBounds, and presumably other, properties of a Box on a TextField do not properly account for non-LEFT alignments. This is likely because TextField applies alignment relative to the texture origin.
I think we can fix this by generating a non-origin-centered hitbox.

Addendum to previous:
It seems to be just the bounds properties that are affected. The hitbox is explicitly managed during text rendering.
Bounds are generated on access from the entity's own position and dimensions. There is no opportunity to modify static data, because it doesn't exist.
I don't think that it's worth changing the whole meaning of bounds just to accommodate a special case. We also have particles, which don't exist within a width/height box at all; text is simply a visual phenomenon that defies box categorization. The hitbox works fine, although you won't be able to do both a custom hitbox and a default bound check.
Long story short: Use hitbox.


address #3053 set ThreadPool creation logging to trace (#3054)

Since this PR caused the history in the main branch and 2.1 to diverge, even though their actual code changes weren't different, I merged them back together again, so both branches have the same HEAD commit. That should remain the case until we start adding 3.0 specific changes to the main branch.


POP_TO not working in android

Version
Tell us which versions you are using:
-react-native-router-flux v3.32.0
-react-native v0.29
Expected behaviour
Actual behaviour
Steps to reproduce
Hello Team I'm using react native router flux but problem is that POP_TO is not working on android but its work on IOs what can I do.
Code --

const reducerCreate = params => {
  const defaultReducer = new Reducer(params);
  return (state, action) => {
    if(action.type == 'REACT_NATIVE_ROUTER_FLUX_BACK_ACTION') {
            Actions.home({type: ActionConst.POP_TO, targetIndex:1});
            //return defaultReducer(state, action);
    }
    return defaultReducer(state, action);
  };
};
 // Home is sceneKey

I am having the same issue


For https://webvr.info/samples/XX-360-video.html update format types

'video/mp4; codecs="mp4v.20.8"' isn't detected as playable by Chrome, but the higher-quality video does play.  This should be a different string.

"mp4v.20.8" is an obsolete codec string. It was added in https://github.com/toji/webvr.info/pull/81/, but it's not clear where it came from.
Running vr_bench_man.mp4 through https://gpac.github.io/mp4box.js/test/filereader.html gives codecs="avc1.42c028,mp4a.40.2".


Disable a11y on tests

Fixes https://github.com/pixiebrix/eslint-config-pixiebrix/issues/220

Tasks:

[x] Verify it

This will only affect one file in the extension, where the rules were already disabled:
https://github.com/pixiebrix/pixiebrix-extension/blob/4a6bdb19347303d492806dccb8a8f02bb0c9ba12/src/components/StopPropagation.test.tsx#L1-L2


Terminal search upwards is broken again

The same as #73459
Will later attach reproducible example

VSCode Version: 1.38.1
OS Version: kubuntu 19.04

Steps to Reproduce:

Search for some word upwards
Then downwards, you will find skipped entries

PS The output contained very long lines and colors

Does this issue occur when all extensions are disabled?: Yes/No

Please provide a more detailed repro.

STR:

Open terminal, cat attached file
Search for opencv upwards, then downwards
If you search downwards you will notice that search finds ...contrib/opencv-3...
Untitled-1.txt

I am seeing some weird behavior:

Searching upwards (enter) is skipping wrapped lines
Searching downwards (shift+enter) is working, but the third wrapped line is not being revealed, this is likely a 1 off error in the logic on whether to scroll up/down

Created upstream issues.

Tracking in https://github.com/xtermjs/xterm.js/issues/2444


(Feature) Unscheduled tasks

Context: .ics, .icalendar

First, thank you a lot. I was searching for something to synchronize my tasks with my calendar and this plugin is just perfect.
This would be even more perfect if the .ics file would also be populated with unscheduled tasks as "todo". I've try with both .ics and .icalendar format but does not seem to load the tasks.
This is compatible with .ics and .icalendar format:
BEGIN:VTODO
SUMMARY:Name
DESCRIPTION:Blablabla
STATUS:NEEDS-ACTION
PRIORITY:0
SEQUENCE:0
END:VTODO

This could be then enabled with a boolean switch in settings.
Thanks in advance

According to code Tasks.ts
You could add a parameter in constructor and then adapt a bit:
bool scheduledTask = (dateMatch === null)
Then use it to lead your process:
  if ( scheduledTask )
    const taskDates = getTaskDatesFromMarkdown(line);
  else
    const taskDates = [ ]; // or VTODO according to ics format?

Maybe there are other places that needs to be adapted.
Full example (not tested):
  const dateMatch = [...line.matchAll(dateRegExp)][0] ?? null;

  // Check wether we need to load dates (if it is a scheduled task or not)
  bool scheduledTask = (dateMatch === null)

  // Extract the Task data points from the matches
  // **Maybe need adaptation here**
  const taskStatus = getTaskStatusFromMarkdown(taskMatch?.groups?.taskStatus ?? '');

  // Task is done and user wants to ignore completed tasks. Bail
  if (taskStatus === TaskStatus.Done && ignoreCompletedTasks === true) {
    return null;
  }

  if ( scheduledTask )
    const taskDates = getTaskDatesFromMarkdown(line);
  else
    const taskDates = [ ]; // or VTODO according to ics format?

  const summary = getSummaryFromMarkdown(taskMatch?.groups?.summary ?? '', howToParseInternalLinks);

  // taskDates could be empty
  return new Task(taskStatus, taskDates, summary, fileUri);

Hi @Blondwolf
Great suggestion.
I think a setting is a great idea, default to off to replicate the existing behaviour.
If the setting is on, the code will append the VTODO block to the end of the iCalendar.
I'm not sure if I need to implement the RELATED-TO property in the VEVENT and VTODO blocks.

Hi again @Blondwolf. I've created a branch and pull request for this one.
https://github.com/andrewbrereton/obsidian-to-ical-plugin/pull/45
Please review and let me know your thoughts.

Ahah I was about to implement it from a fork of your projet. Great yeah, it looks nice. I like your code bro. It is readable.
As I am more a man of test, I need to run it quick locally. I'll come back to you.

Oh well this RELATED-TO field is very interesting.
That would be smart to use it to express dependency over tasks like that in the ical/ics:

[ ]  A task

[ ] A dependant task

Probably not the scope for this ticket issue I think. We should open a new one if you want to work on this.

Okey so, what you've done is ok but you still need to modify a line to include the tasks from your filtering.
Because I don't get any results with unscheduled tasks. So your filter is still active. You are removing the undated elements.
Task.ts line 81:
// This task doesn't have a date. Bail.
if (dateMatch === null) {
    return null;
}

You need to include a variable based on the isIncludeTodo setting you've just added. So you need a new parameter for you TaskFinder. Something like: ignoreUnscheduledTasks.
Just ref the setting with the class parameter
this.taskFinder = new TaskFinder(this.app.vault, this.settings.howToParseInternalLinks, this.settings.ignoreCompletedTasks, this.settings.ignoreOldTasks, this.settings.oldTaskInDays, this.settings.isIncludeTodos);

Then use it:
if (dateMatch === null && ignoreUnscheduledTasks) {

But then you need adaptation. That's why I proposed the code in previous post

Just a comment on your architecture, there are some stuff I don't really understand why you made it such like the custom Mains.ts. I am pretty new to obsidian coding though I have a lot of programming experience so I don't know really what's the standart. Some parts just feels a bit overengineered.
Probably it's better that I wait on this milestone to have the unscheduled tasks completed, then I'll propose a refactored version from my fork and pull request.

Hi @Blondwolf thanks for your input here.
Very good pickup regarding missing VTODO blocks that don't have a date.
I've committed a change to the PR to address this.
Commit that allows for inclusion of Tasks without dates: https://github.com/andrewbrereton/obsidian-to-ical-plugin/pull/45/commits/ba0ab786d9eb0b637d13bfbcd3bc79b932bba323
Whole PR: https://github.com/andrewbrereton/obsidian-to-ical-plugin/pull/45/files
I've got a test Obsidian vault with files and tasks. The generated iCalendar file now includes tasks without dates as VTODO items.

Regarding architecture, this is my first Obsidian plugin so I don't know if I'm doing things the right way or not. The Obsidian devs reviewed it, and gave some suggestions, and approved it so it can't be tooooo bad. It has grown organically though, so I will do a review at some stage. I'd like to have unit tests in place though so I can make changes with more confidence.
In Main, start() is just the main entrypoint. I know the Obsidian sample plugin treats Main similar to how I treat ObsidianIcalPlugin. However I wanted ObsidianIcalPlugin to only deal with the Obsidian integration, and Main to be the entrypoint to the business logic of the plugin.
createTaskFromLine() is a factory method to generate Tasks from strings. I agree that passing settings around like that is not ideal.
Once these current issues/PRs are merged, I plan on storing the settings in a singleton similar to the Logger class. This will fix two issues:

I don't need to pass settings throughout the stack anymore, and
I don't need to ensure that when settings are changed in the UI, the changes propagate through the classes (probably an existing bug).

Please don't get me wrong, the code is pretty clean. Furthermore if it's your first one. Anyway it's nearly impossible to implement the last version directly :p So you did right by implementing it somehow first.
I have something to say for each subject so I opened new tickets.
To keep subject on unscheduled tasks, let me just review it again. I need to quick adapt because I don't use Bun ahah

There is still a problem. You have doubles. events are added as todos as well.
I mean, it depends on what we want to achieve. We could also define many modes or another setting to define if events are added to todos as well.
For this:

[ ] Task
[ ] Planned event 📅 2024-02-06
[x] Finished event 📅 2024-02-05 ✅ 2024-02-05

I have this:
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Andrew Brereton//obsidian-ical-plugin v1.13.0//EN
X-WR-CALNAME:Obsidian Calendar
NAME:Obsidian Calendar
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:9cee450e-863e-42a9-ac5f-dd550629ffa1
DTSTAMP:20240206T000000
DTSTART:20240206
SUMMARY:🔲 Planned event
LOCATION:ALTREP="obsidian://open?vault=Obsidian%20Plugins&file=Test.md":obsidian://open?vault=Obsidian%20Plugins&file=Test.md
END:VEVENT
BEGIN:VEVENT
UID:67573e11-e4d7-471c-876c-9920dd8f15e8
DTSTAMP:20240205T000000
DTSTART:20240205
SUMMARY:✅ Finished event
LOCATION:ALTREP="obsidian://open?vault=Obsidian%20Plugins&file=Test.md":obsidian://open?vault=Obsidian%20Plugins&file=Test.md
END:VEVENT
BEGIN:VTODO
UID:f91a6e2c-3f49-4605-bb0c-a42d016da087
SUMMARY:🔲 Task
STATUS:NEEDS-ACTION
END:VTODO
BEGIN:VTODO
UID:dff64330-a5fe-4612-ae35-8f5183746dba
SUMMARY:🔲 Planned event
DTSTAMP:20240206T000000
DUE;VALUE=DATE:20240206
STATUS:NEEDS-ACTION
END:VTODO
BEGIN:VTODO
UID:50baad81-e4f0-4abf-8304-1d5f34ec76d1
SUMMARY:✅ Finished event
DTSTAMP:20240205T000000
DUE;VALUE=DATE:20240205
COMPLETED;VALUE=DATE:20240205
STATUS:COMPLETED
END:VTODO
END:VCALENDAR

I've implemented it so that every task with a date is a calendar event AND a todo, and every task without a date is only a todo.

I mean, it depends on what we want to achieve.

This is your feature request. What are your requirements?

We could also define many modes or another setting to define if events are added to todos as well.

Does your use-case require this?

 I appreciate but I think that it is better to have something that fulfill the majority of people requirements. Not just mine.

What are your requirements?

Mine is event OR todo separated from each other. Well in fact my first main requirement is already completed by the plugin itself.
Basically I use it to send the .ics directly to my Radicale server. Via a plugin I made for triggering changes on the .ics file, then APIRequest plugin to send the trigger to a NodeRED server which accept my API request, read the .ics file and send it to Radicale via a PUT on caldav.
Having the possibility to synchronize the tasks as well would be nice.

We could also define many modes

So it would be a good idea to define another setting to specify if you want your event to be added as todo as well.
Or you could use an enum as setting (or a number if not allowed) to specify between 3-4 differents modes. Maybe it is better to have only 2 boolean if you are not so experienced.

Thank you for explaining your use-case, it is helpful. I'm glad to hear that your main requirement is completed. Throughout my life, something I try to avoid doing is building solutions for the assumed silent audience. Oftentimes I have found that it makes code more complex, and even more often, the assumed use-case does not exist in reality. (i.e.: ASSUME: makes an ASS of U and ME 😄). This is why I am a bit hesitant to implement something for a perceived problem.
With that said, I think it is rather simple to add a new setting which asks the user to decide if they want tasks with dates added as a TODO, or just tasks without dates added as a TODO. And again quite simple to consider this setting when building the iCalendar file. For example:
If Add tasks as TODO items to your calendar is true then show this setting:
Do you want to include only tasks without dates as TODO items, or all tasks?
Options:

Only tasks without dates (default)
All tasks

This is a good YAGNI feeling that you have. You should indeed always ask yourself if this is necessary.
Furthermore comparing to old softwares architectures (used to be closed, non-standardized), where we have now plugins, API, and more standards, that leads to a way better program interoparability. Simply say it is way easier to make softs communicate so enhance modularity.
In this case, this would be a bit hard to add a wrapper or an extension working with your plugin. Well it would possible to re-read the entire calendar file but that would be a bit an overkill. In this case, you are indeed generating the .ics file so it still make sens for me.

What are your thoughts on this issue? Should I:

Mark it as done and close it,
Add the additional setting Do you want to include only tasks without dates as TODO items, or all tasks?, or
Something else.

Egoistically I would say 2)
But please, this is not urgent, so prioritize your needs

This has been in 1.17.0

I need to test it but it looks damn awesome. Thank you a lot. Sorry I haven't been able to help you since.


Sentence Tokenizer Split sentence on Terminal Puntuation.

Detailed description
Currently the Crfcut engine for the sentence tokenize part does not split the sentence correctly for terminal punctuations, for enders it is working fine, but when it comes to split using terminal punctuation commonly '.' (full stop), it's still creating a issue. I tried with different engines too, but i could not get the desired output for the sentence tokenization. Since also came across it is trained on TED Dataset so it may certainly have not came across that maybe.
Context
Trying to look for this fix to handle formatting of the sentence after translation.
Also, would be useful for those looking for similar use case.
Possible implementation
This is what currently i was tring for my use case. Below is the code provided :
for idx, _ in enumerate(toks):
if(toks[idx].strip().endswith(('!', '.', '?'))):
labs[idx] = "E"
    #Spaces would no longer would be treated as end of the sentences.
    elif(toks[idx].strip() == ""):
        labs[idx] = "I"

Your environment

PyThaiNLP version: 3.1.1
Python version: 3.10
Operating system and version (distro, 32/64-bit): 64 bit
More info (Docker, VM, etc.):

Here i have attached all the cases where, crfcut fails to split using terminal punctuations in some cases. But since terminal punctuations marks the end of sentence, it should always split, but many a cases it is not working, current fix to it i have added is given below :
def segment(text: str) -> List[str]:
"""
CRF-based sentence segmentation.
:param str text: text to be tokenized to sentences
:return: list of words, tokenized from the text
"""
if isinstance(text, str):
    toks = word_tokenize(text)
else:
    toks = text
feat = extract_features(toks)
labs = _tagger.tag(feat)
labs[-1] = "E"  # make sure it cuts the last sentence

**
for idx, _ in enumerate(toks):
if(toks[idx].strip().endswith(('!', '.', '?'))):
labs[idx] = "E"
    #Spaces would no longer would be treated as end of the sentences.
    elif(toks[idx].strip() == ""):
        labs[idx] = "I"

**
sentences = []
sentence = ""

for i, w in enumerate(toks):
    sentence = sentence + w
    **if labs[i] == "E" and sentence != '':**
        sentences.append(sentence)
        sentence = ""

return sentences


Error message floating out of screen IOS/Android Fix

Details

This PR fixes the issue of error message float out of screen on android and ios
Fixed Issues

$ https://github.com/Expensify/App/issues/11488
PROPOSAL: https://github.com/Expensify/App/issues/11488#issuecomment-1301110463
Tests

Go to Workspace > connect bank account
Check error message
Error message should not be floating out of screen

[x] Verify that no errors appear in the JS console

QA Steps

Go to Workspace > connect bank account
Check error message
Error message should not be floating out of screen

[x] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] iOS / native
[x] Android / native
[x] iOS / Safari
[x] Android / Chrome
[x] MacOS / Chrome
[x] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product was added in all src/languages/* files
[x] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

[ ] I have verified the author checklist is complete (all boxes are checked off).
[ ] I verified the correct issue is linked in the ### Fixed Issues section above
[ ] I verified testing steps are clear and they cover the changes made in this PR

[ ] I verified the steps for local testing are in the Tests section
[ ] I verified the steps for Staging and/or Production testing are in the QA steps section
[ ] I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[ ] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)

[ ] I checked that screenshots or videos are included for tests on all platforms
[ ] I included screenshots or videos for tests on all platforms
[ ] I verified tests pass on all platforms & I tested again on:

[ ] iOS / native
[ ] Android / native
[ ] iOS / Safari
[ ] Android / Chrome
[ ] MacOS / Chrome
[ ] MacOS / Desktop

[ ] If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
[ ] I verified proper code patterns were followed (see Reviewing the code)

[ ] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
[ ] I verified that comments were added to code that is not self explanatory
[ ] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[ ] I verified any copy / text shown in the product was added in all src/languages/* files
[ ] I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[ ] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[ ] I verified the JSDocs style guidelines (in STYLE.md) were followed

[ ] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[ ] I verified that this PR follows the guidelines as stated in the Review Guidelines
[ ] I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
[ ] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[ ] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[ ] If a new component is created I verified that:

[ ] A similar component doesn't exist in the codebase
[ ] All props are defined accurately and each prop has a /** comment above it */
[ ] The file is named correctly
[ ] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[ ] The only data being stored in the state is data necessary for rendering and nothing else
[ ] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[ ] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[ ] All JSX used for rendering exists in the render method
[ ] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[ ] If a new CSS style is added I verified that:

[ ] A similar style doesn't already exist
[ ] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)

[ ] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[ ] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[ ] I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

@syedsaroshfarrukhdot Link issues properly

@syedsaroshfarrukhdot Link issues properly

Just updated them.

🚀 Deployed to staging by @marcochavezf in version: 1.2.26-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

@syedsaroshfarrukhdot  How to trigger this error message in Desktop, mWeb, iOS and Android. We can't get past without validating the account in NewDot? Web we were able to test following these steps.

go to OldDot
create account
create free policy
click on it
go to workspace
connect bank account

in mweb and apps we are blocked with this KI #11245

Oh interesting, I thought it was possible to test with the same account on different platforms but yeah seems it's not possible to log in without validating the account. I didn't verify that.
@syedsaroshfarrukhdot @Santhosh-Sellavel is there a friendly user way to test this on other platforms? Or was it only possible by forcing it in the code? The change looks good, but can we confirm this scenario is reproducible on staging / production only on web?

@marcochavezf Yes, it is interesting it was only reproducible by forcing it in the code.  I believe as it is not possible to login in the app without without validating the account. I believe we might never encounter this error in staging / production until we allow user to login without being verified which is not the case for now.

Maybe we should mark this as no QA, for now!

Sounds good - checking this off then ✅

🚀 Deployed to production by @yuwenmemon in version: 1.2.26-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅


Replace DateTime usage with DateTimeOffset

This commits are related to Issue #5786

Actually It's not a bug but the warning that dotnet shows suggesting that you replace DateTime with DateTimeOffset to better handle date and time values, particularly when working with different time zones or when precision is needed regarding the exact offset from Coordinated Universal Time (UTC).
Your code is already in the right direction since you're creating a DateTimeOffset object. However, directly using DateTime to create a DateTimeOffset object isn't wrong per se, but there are better ways to do it.

@zandiarash Thanks a lot for submitting this PR, but we do not file a PR without first discussing the problem in a separate issue. Basically, the PR is not the place to investigate the issue for us. So if you can please submit the issue for this problem too.
btw, we are already aware of the problem and have plans to fix it, but we very welcome external contributions like this too.

@zandiarash Thanks a lot for submitting this PR, but we do not file a PR without first discussing the problem in a separate issue. Basically, the PR is not the place to investigate the issue for us. So if you can please submit the issue for this problem too. btw, we are already aware of the problem and have plans to fix it (we have a few related issues/PRs), but we very welcome external contributions like this too.

Of course!
Initially, I wished to spare you the effort of reading the issue about a suggestion which isn't truly a bug.
Thank you for your guidance and clarity on the procedure. I understand the importance of discussing the issue in a separate forum before submitting a PR. I'll ensure to raise an issue detailing the problem shortly.
I'm glad to hear that the team is already working on it, and I'm always eager to contribute where I can. Let me know if there's any other way I can assist or align my contributions better with the team's strategies.

@zandiarash Wow, great job, thanks a lot 🚀


[FEATURE]: Support nodejs 20

When installing with yarn I am getting this (Windows, nodejs v 20):
error<EMAIL_ADDRESS>The engine "node" is incompatible with this module. Expected version ">=14 <=18". Got "20.4.0"
error Found incompatible module.

I know I can suppress this messge by --ignore-engines flag. I am guessing this error comes from dependency on https://github.com/DeepLcom/deepl-node - but not sure about it. Is that the problem? What would be the best way to fix it?

Thanks for the quick response on this. Much appreciated 👍


Tree listing shows only first 5 commit messages

When browsing a repository that contains more than five folders / files, only the first five listings will show the commit message and commit date.
I'm using the latest code from the master branch.

Sorry to say that this doesn't repro here.
Those messages / dates should come from an AJAX request to http://[server]/Repository/Tree/[repo guid] - if you just do an ordinary browser GET on that URL you get the page without the details, then the page itself requests it again, and the AJAX request is served up with the details in it.  (Not my idea...)
Would you mind having a look at the AJAX request with your browser's dev tools, and see if all the messages/dates are correctly included - if they are, then it's a client-side problem, if they're not it's a server-side problem.
There may also be something in the data which is causing the browser to choke - this may be apparent if you have the browser dev-tools console open while fetching the page.

I was able to reproduce on my end.  When browsing a repo, a couple commit messages are displayed, then the dev tools show the following error:
2261c144-afc8-4b7a-96cd-fd73350fbfea?encodedPath=AerialViewer%2FAerialViewer:484 Uncaught TypeError: Cannot read property 'CommitMessage' of undefined
at HTMLTableRowElement. (2261c144-afc8-4b7a-96cd-fd73350fbfea?encodedPath=AerialViewer%2FAerialViewer:484)
at Function.each (jquery-1.4.4.js:652)
at init.each (jquery-1.4.4.js:276)
at Object.detailsLoaded [as success] (2261c144-afc8-4b7a-96cd-fd73350fbfea?encodedPath=AerialViewer%2FAerialViewer:483)
at Function.handleSuccess (jquery-1.4.4.js:6168)
at XMLHttpRequest.xhr.onreadystatechange (jquery-1.4.4.js:6002)
No other commit messages are shown after the above error occurs.
I found another repo that produced a similar error:
Uncaught TypeError: Cannot read property 'CommitMessage' of undefined
at HTMLTableRowElement. ...

For some reason which I'm not able to see at the moment just by looking at the code, it must be serving up a different number of files in the summary information which draws the table to when it serves up the detail.
Would you be able to share any more information which could help here - ideally the repository (could be just the .git directory I think, no actual files), or the JSON which is returned by the AJAX call to the Tree method.  At the very least you could check the Files array in the JSON and see if it has some missing items, or if it just has the first few items and then stops.
Without a repro here it's really hard to know how to proceed.  I have tried various odd/blank commit messages, and it's not that.

Having repro'd something which looks very like this, I realise that it's not showing a truncated list of messages, it's showing the messages which belong with the level above in the file hierarchy.
I have just committed a fix for this in 656e58b - once this has built, would you be able to try it?

I have noticed that the commit message shown in the folder / file listing was not always the correct message.  You may be on to something when you said it belonged to the level above.  I look forward to testing this code change.

@KY4U I am just making a public fool of myself by committing things onto Master which break the tests...  When I've finished with this then you should be able to try it...

It appears that I'm getting the correct commit messages (thank you), however, there is some serious lag time during the second jquery request for that information.

Retrieval time for the commit information ranges from 3-30+ seconds.  I haven't determined a contributing factor for the range difference.

I think it's the length of time which that operation can take which lead to someone in the past splitting it into the separate page request followed by an AJAX request to get the details.
I don't fully understand GetTreeModelsWithDetails, but I can believe it has some sort of n^2-ish behaviour, perhaps if you have a directory with a large entries of files that is the child of a directory with a large number of entries?
I can't find anything here which looks slow as I browse through it - do you see any relationship between the size of directories and the delays?

I thought that to be the case but here are some stats:

4 subdirs and 3 files: 5 seconds (60 commits in repo)
11 subdirs and 20 files: 6 seconds (60 commits in repo)
0 subirs and 20 files: 1.5 seconds (60 commits in repo)
9 subdirs and 60 files: 1.5 seconds  (3 commits in repo)
1 subdir and 1 file: 6 seconds (660 commits in repo)
4 subdirs and 4 files: 9.5 seconds (660 commits in repo)
6 subdirs and 60 files: 15 seconds (660 commits in repo)
20 subdirs and 50 files: 6 seconds (70 commits in repo)
1 subdir and 20 files: 12 seconds (160 commits in repo)

So far nothing is jumping out at me that might be causing the delay.  I was leaning towards the number of commits in the repo but my results aren't remaining consistent.

@KY4U Those are hardly large repos/directories...
I think the only thing to do would be to run a profiler or at least add some StopWatch prints to GetTreeModelsWithDetails to see where the time's going.
I don't ever see anything more than a couple of seconds delay here.

I'll poke around a bit with the code to see if I can narrow down the issue.  The main hurdle seems to be cleared.  The commit messages are displaying correctly, albeit somewhat slowly at times.  That could easily be something on our end.

Time for an SSD...