Support JUnit 4 RunListeners

Expected Behavior
I'd like to be able to include my own JUnit RunListener classes for starting up and shutting down resources that are used across test suites to massively reduce resource setup overhead on test suites, the same way that it's supported in Maven surefire.
Current Behavior
There's no way to setup a run listener using Gradle
Context
This affects me in that, for testing, we used multiple "outside" resources like database, message queue, mail, etc. services, and we embed those for more real world validation testing.
As those services tend to take time to startup and shutdown, we setup test suites and a run listener that initializes such services only once, then executes all tests, and shuts them down, otherwise, the services get startup and shutdown per test, which consumes a lot of time and resources.
Your Environment
This is executed both in developers local machines with MacOS, Windows and Linux, and when executed by the CI server (Jenkins) it's tends to execute a lot of build pipelines in parallel, so this is where we notice the resource usage the most.

I'd like to explore the use cases in more depth.
Here's a list of questions and concerns that came up in internal discussions

What do these custom listeners provide that current TestListeners do not?
Why aren't these custom listeners added in the code, making them available in the IDE?
What impact will running tests in parallel have on these listeners?

What impact will the forkEvery flag have?

Will this strategy be compatible with JUnit 5?

It's also worth noting that a very similar use case was submitted in https://github.com/gradle/gradle/pull/701

@w25r the problem is that currently there are no way to use custom reporting (for example - Allure Framework) with JUnit 4 and Gradle. And there are no way to get all the information about the test using TestListeners (they are kind of designed to only get list of the tests and their statuses). Allure can catch a much more such as steps, attachments, test parameters, custom groups, test fixtures etc.

@baev I see that you've already contributed to an example using Allure with Gradle.  I think that is a valid workaround (if not a better alternative) to customizing test behavior in a build script.
Another possibility is performing these changes in a Gradle plugin that allows custom listeners to be added.  That greatly reduces the risk, as only consumers of the plugin would be affected by the changes.
If we still do want to move forward with a change to Gradle proper, the next step we need is a well thought out Design Spec that addresses the concerns outlined above.

I see that you've already contributed to an example using Allure with Gradle. I think that is a valid workaround (if not a better alternative) to customizing test behavior in a build script.

This requires to configure Runner for all the tests. Imagine to patch all custom runners (Parameterized, SpringRunner etc) to add the listener to tests. The problem is that we can not migrate test projects with thousands of tests from Maven to Gradle because of that.
The other workaround is use AspectJ to catch execution of internal JUnit methods and add the listener in there. But this smells bad

Another possibility is performing these changes in a Gradle plugin that allows custom listeners to be added. That greatly reduces the risk, as only consumers of the plugin would be affected by the changes.

What plugin do you have in mind? Do you mean to create a brand new plugin?

If we still do want to move forward with a change to Gradle proper, the next step we need is a well thought out Design Spec that addresses the concerns outlined above.

I can do all the work, this issue is kind of blocker for us.

What plugin do you have in mind? Do you mean to create a brand new plugin?

Yes, a brand new plugin would be perfect.  apply plugin: 'junit-listeners' or something to that effect.  Some investigation would be required to see if there is an opportunity to override existing behavior.  Ideally, you could swap out the existing runner with the one you submitted in the pull request, but do it as a plugin.
If there doesn't seem to be a way to get what needs to be done in plugin form, then a design spec would be great.

If there doesn't seem to be a way to get what needs to be done in plugin form

I already tried this but no luck.

@baev Where you able to solve this issue? Where you looking to simply add an extra listener or override the OFTB Gradle one? I've come across a similar issue and I'd be interested to know how you have got on.

I  need to add one more JUnit listener. The only way I know is using the following aspect:
@Aspect
public class AllureRunListenerAspect {

    private static AllureRunListener allureRunListener = new AllureRunListener();

    @Pointcut("execution(void org.junit.runners.ParentRunner.run(org.junit.runner.notification.RunNotifier))")
    public void run() {
    }

    @Around("run()")
    public void run(ProceedingJoinPoint pjp) {
        Object[] args = pjp.getArgs();
        RunNotifier notifier = (RunNotifier) args[0];
        notifier.removeListener(allureRunListener);
        notifier.addListener(allureRunListener);
        notifier.fireTestRunStarted(Description.createSuiteDescription("Tests"));

        try {
            pjp.proceed(args);
        } catch (Throwable throwable) {
            throwable.printStackTrace();
        }

        notifier.fireTestRunFinished(new Result());
    }
}

I tried to create a brand new Gradle Plugin that adds listener, but can't find the way to do it.

Is there any chances to get this processed?

+1

@kcooney @marcphilipp could you please help us with the issue? Currently there is no way to add a listener to JUnit4 in Gradle projects. Since we decided to not merge SPI support for listeners https://github.com/junit-team/junit4/pull/1122 to JUnit 4 that is blocker for us

@baev as stated before, we'd like to see a design spec that addresses the aforementioned issues before this can be addressed.
How close did you get on the plugin side?  If you push your repo, we could take a look at it and may be able to resolve your issues (or confirm that it can't be done from a plugin)

The new JUnit Platform supports adding listeners via ServiceLoader. It can run old JUnit 4 ("Vintage") and new Jupiter tests. Is that a viable alternative for you?

The new JUnit Platform supports adding listeners via ServiceLoader. It can run old JUnit 4 ("Vintage") and new Jupiter tests. Is that a viable alternative for you?

Nope, some our users can't migrate to JUnit 5 (banks, legacy projects etc)

How close did you get on the plugin side? If you push your repo, we could take a look at it and may be able to resolve your issues (or confirm that it can't be done from a plugin)

I didn't find a way how to override logic in JUnitTestClassExecuter, so there is no code at all

as stated before, we'd like to see a design spec that addresses the aforementioned issues before this can be addressed.

I'm not familiar with your dev process. Can you link some articles about design specs etc?
BTW I can do all the work but I need to be sure that this change gonna be merged some day. Also I don't know about your feature plans about JUnit 5 support in Gradle, how this will work together with JUnit4 etc. I pretty much sure that there are so many people that will use JUnit 4 for next few years and will not migrate to JUnit Platform, so It would be useful to have an ability to configure RunListeners anyway.

Sorry for the late response. A few thoughts:
First of all, the JUnit RunListener API was intended to be used to notify code about the state of running tests (to drive a UI, produce an XML report, etc). It wasn't intended to be used to change the behavior of the running tests. By using it for this new purpose, you might get some confusing behavior. For example, if you do work in testFinished() that could throw an exception, the listeners could be notified about the failure after they were notified that the test was finished, and some listener notifications might not expect this. I realize that you have to work with the APIs you have available, but I thought I should mention this.
Secondly, I'm no longer comfortable with the idea of using an SPI interface to register listeners in JUnit. Doing so would allow any code in your classpath to add listeners, even if those listeners cause problems or are even hostile to your development environment. I could be convinced otherwise, but currently I'm thinking that if JUnit4 supported registering listeners via ServiceLoader there would need to be some way for a test in package com.example.project to opt-in for a listener that lives in org.evil.
The Gradle config could be one way to opt-in, but then you would only get the behavior if you run the tests in Gradle.
I'm confused about the concern about JUnit5. The users would not need to change their tests to use JUnit5. JUnit5 provides a newer API for running JUnit4-style ("Vintage") and JUnit5-style ("Jupiter") tests (I believe JUnit3-style tests would run using the JUnit4 code, so those would be supported, too).  I think Marc was suggesting that Gradle would eventually migrate to using the JUnit5 APIs to run JUnit tests, and you could take advantage of those APIs (which would also not require having your users upgrade their JUnit jars, since JUnit5 uses different maven packages).

the JUnit RunListener API was intended to be used to notify code about the state of running tests (to drive a UI, produce an XML report, etc). It wasn't intended to be used to change the behavior of the running tests.

I agree, so all we need is produce extended XML report (steps, parameters, attachments, history, retries etc).

I'm no longer comfortable with the idea of using an SPI interface to register listeners

I got your point. The main idea of this conversation is that currently there no way to add listeners to JUnit using Gradle. And since Maven is kind of dead, our users start to migrate from Maven to Gradle. And they can't use Allure any more. That is the problem
I am totally fine with using JUnit 5 platform inside Gradle to run the tests as long as users will not require to change the code of tests. The only thing is that it would be so nice to get the problem resolved as soon as possible.

the JUnit RunListener API was intended to be used to notify code about the state of running tests (to drive a UI, produce an XML report, etc). It wasn't intended to be used to change the behavior of the running tests.

I agree, so all we need is produce extended XML report (steps, parameters, attachments, history, retries etc).

Sorry for the confusion, @baev. @raulgd (who opened this issue) indicated that he wanted to shutdown resources in the listener. If all Allure needs is a test run report, then RunListener would work, and doing it in Gradle seems like the right path. It looks like TestListener might work as well. It might help to let the Gradle team know what you need to do that cannot be done with TestListener

As I mentioned before TestListener designed to only get list of the tests and their statuses. Allure requires much more (an example of our RunListener you can find here).

@baev I'm sure I'm missing something, but I still don't quite see what you need that TestListener doesn't provide. You are overriding testStarted() (which apparently maps to beforeTest()), testFinished() (== afterTest()) and testFailure() (== afterTest()). The fields you access from Description seem to have equivalents in Gradle's TestDescriptor (assuming you can map from the class name to the class). Is the problem that Gradle's TestListener is running in a different process than the test, and you need something in the same JVM?

Is the problem that Gradle's TestListener is running in a different process than the test, and you need something in the same JVM?

Yep, we are using ThreadLocal to catch information about steps and attachments and reflection to get some metadata from test class/method (an example from annotations).

I think it's an intriguing idea to try to leverage JUnit 5's ServiceLoader mentioned by @marcphilipp.  It looks like you can already use JUnit 5 with Gradle, so it shouldn't be a problem there.  This is probably the best bet for getting this problem solved in the short term.
If that doesn't work, design specs can be found here.  We already discussed some of the use cases and concerns that should be addressed in the design.  As for promising that it's going to be merged someday, well, that is impossible to guarantee.  But if the Gradle team determines that it is a safe, well-thought, well-tested change, then we are likely to merge it.

shall I create an separate PR with design docs or continue the work in #1725?

A new one would probably be better, and we can reference this issue and the old PR.

I believe the primary issue @baev refers to is the absence of TestDescriptor.getAnnotations().
It is present in JUnit's Description class, but gradle ignores this important piece of information when constructing DefaultTestDescriptor.
It is quite common to use test annotations for reporting purposes (allure is a great example).
Is there any chance we can get them into the TestDescriptor?

Hey guys, any updates on this issue?

I posted a potential solution in the Help/Discuss forum that could help JUnit 4 busers who need to attach RunListener providers to their test suites.
https://discuss.gradle.org/t/how-to-attach-a-runlistener-to-your-junit-4-tests-in-gradle/30788
The post describes how to  configure a Gradle project to use the JUnit Foundation library to attach RunListener classes to JUnit 4 test suites. Listeners are specified in a ServiceLoader provider configuration file, and the library automatically attaches them.

Thanks @sbabcoc, I'll try that out!

@pelizza Let me know if you run into any issues. I've fixed all of the problem I encountered myself, but my scenarios are very limited. Also, I've tried to provide clear, complete documentation, but it's likely that more details would be helpful here and there. Feedback is welcomed!

@oehme I think the information I posted will be helpful to many folks still using JUnit 4. The library I refer to enables timeout management, automatic retry of failed tests, and event notifications in Gradle project tests. Where is the best place to publicize this information?

The forum or a blog post, so people will find it when they google for it.

@sbabcoc I attempted to utilize this solution in my project and it is not working. I am not seeing any mention of the RunListener being picked up as the execution goes through it's processing. Any assistance would be welcome.

@cah-daniel-adams01 The current release is 9.4.2. Try this first. Since this is a Gradle forum, we should probably continue any further discussion on the JUnit Foundation project repo.

I can report a working RunListener using JUnitFoundation as reported by @sbabcoc. E.g. following https://stackoverflow.com/questions/54450475/junit-listener-configuration-in-gradle
This works fine for testRunStarted calls.
However: To also get the calls to testRunFinished I needed to switch to 'useJUnitPlatform()' (using the vintage engine).

Yes, JUnit Foundation is implemented specifically to support JUnit 4. The testRunStarted notification is issued by JUnit Foundation when each RunListener is attached, but the testRunFinished notification comes from the JUnit 4 API.

any update on this request?


Add panel-based GUI

(using my sigslot mechanism)

@jsignell , thought you'd appreciate this new version

Some next-level questions:

some backends can derive kwargs from the url; should they be interpreted and fill in the kwargs box automatically, or just used as-is?
since we now support chaining, how would you do something like "browse the contents of the currently selected zip file" or pick a protocol (e.g., dask::s3) other than the simple list?
it would be good to optionally show the docstring and/or signature of the currently-selected protocol's backend somewhere


pull request for e2e test at 11/23/2016 01:26:09

:white_check_mark: Validation status: passed

File
Status
Preview URL
Details

optest/docs/updated_at.md
:white_check_mark:Succeeded

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


Use new diff option in drive.clear

Depends on https://github.com/holepunchto/hyperblobs/pull/16

@mafintosh Ready!


Ignoring a player causes some messages from other players to be hidden from the chatbox.

Sometimes, when I add someone to my ignore list, certain messages from other players will still appear above their heads, but not be in the chatbox. This isn't all messages from these players either, only some, seemingly randomly.

Are you sure these are typed chat messages and not from items etc?

This looks like jagex bug, we do not do anything with ignored players


ROX-16811: Create a custom RHEL updater

Description

Child PR for CVSS: https://github.com/stackrox/stackrox/pull/9138
Child PR for unfixed opneshift vulns: TBD

Diff between ClairCore and the custom RHEL updater:
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: coalescer.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: coalescer_test.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: distributionscanner.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: distributionscanner_test.go
Only in /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel: doc.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: dockerfile
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: ecosystem.go
Only in /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/internal/common: charset.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/internal/common: updater.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/internal: containerapi
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: matcher.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: matcher_test.go
diff -bdur /home/jvdm/wt/github.com/quay/claircore/main/rhel/parser.go /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/parser.go
--- /home/jvdm/wt/github.com/quay/claircore/main/rhel/parser.go	2023-11-03 20:16:13.282361986 -0300
+++ /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/parser.go	2023-12-19 16:49:22.483207860 -0300
@@ -10,10 +10,9 @@
 	"github.com/quay/zlog"

 	"github.com/quay/claircore"
-	"github.com/quay/claircore/internal/xmlutil"
 	"github.com/quay/claircore/pkg/ovalutil"
-	"github.com/quay/claircore/rhel/internal/common"
 	"github.com/quay/claircore/toolkit/types/cpe"
+	"github.com/stackrox/rox/scanner/updater/rhel/internal/common"
 )

 // Parse implements [driver.Updater].
@@ -28,7 +27,7 @@
 	defer r.Close()
 	root := oval.Root{}
 	dec := xml.NewDecoder(r)
-	dec.CharsetReader = xmlutil.CharsetReader
+	dec.CharsetReader = common.CharsetReader
 	if err := dec.Decode(&root); err != nil {
 		return nil, fmt.Errorf("rhel: unable to decode OVAL document: %w", err)
 	}
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: repositoryscanner.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: repositoryscanner_test.go
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel: rhcc
diff -bdur /home/jvdm/wt/github.com/quay/claircore/main/rhel/rhel.go /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/rhel.go
--- /home/jvdm/wt/github.com/quay/claircore/main/rhel/rhel.go	2023-12-06 17:36:04.217752009 -0300
+++ /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/rhel.go	2023-12-19 16:49:22.483207860 -0300
@@ -22,6 +22,7 @@
 var (
 	_ driver.Updater      = (*Updater)(nil)
 	_ driver.Configurable = (*Updater)(nil)
+	repositoryKey                     = "rhel-cpe-repository"
 )

 // Updater fetches and parses RHEL-flavored OVAL databases.
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/testdata: com.redhat.rhsa-RHEL8.xml
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/testdata: layer-with-cpe.tar
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/testdata: layer-with-embedded-cs.tar
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/testdata: layer-with-no-cpe-info.tar
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/testdata: Red_Hat_Enterprise_Linux_5.xml
Only in /home/jvdm/wt/github.com/quay/claircore/main/rhel/testdata: releasefiles
diff -bdur /home/jvdm/wt/github.com/quay/claircore/main/rhel/updaterset.go /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/updaterset.go
--- /home/jvdm/wt/github.com/quay/claircore/main/rhel/updaterset.go	2023-10-12 03:58:11.221035023 -0300
+++ /home/jvdm/wt/github.com/stackrox/stackrox/rhscores/scanner/updater/rhel/updaterset.go	2023-12-19 16:49:22.535208026 -0300
@@ -12,7 +12,7 @@
 	"github.com/quay/zlog"

 	"github.com/quay/claircore/libvuln/driver"
-	"github.com/quay/claircore/rhel/internal/pulp"
+	"github.com/stackrox/rox/scanner/updater/rhel/internal/pulp"
 )

 // DefaultManifest is the url for the Red Hat OVAL pulp repository.

Checklist

[ ] Investigated and inspected CI test results
[ ] Unit test and regression tests added
[ ] Evaluated and added CHANGELOG entry if required
[ ] Determined and documented upgrade steps
[ ] Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)

If any of these don't apply, please comment below.
Testing Performed
Here I tell how I validated my change
TODO(replace-me)
Use this space to explain how you validated that your change functions exactly how you expect it.
Feel free to attach JSON snippets, curl commands, screenshots, etc. Apply a simple benchmark: would the information you
provided convince any reviewer or any external reader that you did enough to validate your change.
It is acceptable to assume trust and keep this section light, e.g. as a bullet-point list.
It is acceptable to skip testing in cases when CI is sufficient, or it's a markdown or code comment change only.
It is also acceptable to skip testing for changes that are too taxing to test before merging. In such case you are
responsible for the change after it gets merged which includes reverting, fixing, etc. Make sure you validate the change
ASAP after it gets merged or explain in PR when the validation will be performed.
Explain here why you skipped testing in case you did so.
Have you created automated tests for your change? Explain here which validation activities you did manually and why so.
Reminder for reviewers
In addition to reviewing code here, reviewers must also review testing and request further testing in case the
performed one does not seem sufficient. As a reviewer, you must not approve the change until you understand the
performed testing and you are satisfied with it.

It may also be worth looking at which files/tests we don't really care about because we didn't touch so we can minimize the size of this change (and the repo once this is merge)

It may also be worth looking at which files/tests we don't really care about because we didn't touch so we can minimize the size of this change (and the repo once this is merge)

It's fine leaving some garbage behind: It's easier to compare with ClairCore, and it will be deleted in the next release (or eventually).


Drop support for node <8

This is what's causing the coverage to change so drastically in #66 - b/c the tests fail against node@4, there's no coverage!

Coverage remained the same at 94.636% when pulling a7c2b0151726bf5ed0f2f61b3d8d3985ee1a3101 on G-Rath:patch-1 into aba2520926580d7ea01090004118aa8ef5b508a6 on jessedc:master.


re-attached tmux windows stop accepting normal keyboard input

I have an issue, which I am not sure if it's a wslbridge issue or mintty's, it seems to be some console connection problem.
issue: re-attached tmux windows stop accepting normal keyboard input
Versions:
wslbridge version: 0.2.1 cygwin64
mintty 2.7.6 (x86_64-pc-cygwin)
putty 0.68
winver:  version 1703 (OS build 15063.138) aka Windows 10 Creators Update
ubuntu is updated:
$  lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 16.04.2 LTS
Release:        16.04
Codename:       xenial
$ tmux -V
tmux 2.1
How to re-create

start with WSL not running (no bash.exe running in TaskManager)
Open 2 mintty instances
in the first mintty console, start with

tmux -f /dev/null new

to create a new tmux session, you can use Ctrl+b c to create new windows, switch between windows, run some commands in tmux windows..., tmux seems working fine
note that I use -f /dev/null instead of using my .tmux.conf to avoid configuration issues.
4. now use windows' close button ([x]) on mintty's window title bar to close the first mintty console, to emulate that the mntty connection gets lost
5. now switch to the second mintty console, run
tmux -f /dev/null attach

to re-attach the running tmux session.
6. from the re-attached tmux window, type some commands. after several seconds, the tmux window stops accepting normal key input, the tmux process itself does not die and mintty still connects to WSL because you can still use Ctrl+d to exit the window, or Ctrl+b 0/1 to switch to another tmux window. Any new tmux window created by Ctrl+b c also does not accept normal key input.
7. the tmux session stays in this state till I kill the tmux server or I quit WSL totally by closing all mintty consoles.
I tried the same test with
. Microsoft's original "Bash on Ubuntu on Windows" cmd console: 2 consoles, create tmux session from the first console, close the first console and re-attach tmux session from the second console, no issues.
. running sshd server and connect with putty (0.68), 2 connections, create tmux session from the first ssh connection, kill the first connection by close putty window without disconnecting, then re-attach from the second connection, no issues.

I'm running the same versions of software components, and I'm unable to reproduce this issue.
It sounds like wslbridge is still successfully transferring data between mintty and the WSL pty, but tmux isn't able to write input into its session ptys?

it seems like tmux lost stdin/stdout.
I just turned on tmux verbose log, there were millions of these lines:
1492880625.964822 @0 active pane not changed
1492880625.964852 event dispatch enter
1492880625.964922 %0 has 923 bytes
1492880625.964952 %0 backing off (/dev/pts/3 1723 > 512)
1492880625.964981 event dispatch exit

when tmux was in this weird state.


Bug avoid ignore rf multiple times (#44408)

avoid ignore rf multiple times
When an instance is wake_up_by_downstream, it will destroy the logic of first closing the instance that should build hash table and then closing other instances, instance that should build hash table during the process of inserting data into rf, this instance may find that rf is ignored, causing dcheck to fail.
 F20241121 12:01:02.245405 9832 runtime_filter.cpp:395] Check failed: !is_ignored()

3# raise at ../sysdeps/posix/raise.c:27
4# abort at ./stdlib/abort.c:81
5# 0x00005603CFDF778D in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
6# 0x00005603CFDE9DCA in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
7# google::LogMessage::SendToLog() in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
8# google::LogMessage::Flush() in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
9# google::LogMessageFatal::~LogMessageFatal() in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
10# doris::RuntimePredicateWrapper::insert_fixed_len(COW<doris::vectorized::IColumn>::immutable_ptr<doris::vectorized::IColumn> const&, unsigned long) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/exprs/runtime_filter.cpp:395
11# doris::RuntimePredicateWrapper::insert_batch(COW<doris::vectorized::IColumn>::immutable_ptr<doris::vectorized::IColumn> const&, unsigned long) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/exprs/runtime_filter.cpp:431
12# doris::IRuntimeFilter::insert_batch(COW<doris::vectorized::IColumn>::immutable_ptr<doris::vectorized::IColumn>, unsigned long) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/exprs/runtime_filter.cpp:992
13# doris::VRuntimeFilterSlots::insert(doris::vectorized::Block const*) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/exprs/runtime_filter_slots.h:146
14# doris::pipeline::HashJoinBuildSinkLocalState::close(doris::RuntimeState*, doris::Status) in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
15# doris::pipeline::DataSinkOperatorXBase::close(doris::RuntimeState*, doris::Status) in /mnt/ssd01/doris-branch40preview/NEREIDS_ASAN/be/lib/doris_be
16# doris::pipeline::PipelineTask::close(doris::Status) at /home/zcp/repo_center/doris_branch-3.0/doris/be/src/pipeline/pipeline_task.cpp:487

What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)

Test

[ ] Regression test
[ ] Unit Test
[ ] Manual test (add detailed scripts or steps below)
[ ] No need to test or manual test. Explain why:

[ ] This is a refactor/code format and no logic has been changed.
[ ] Previous test can cover this change.
[ ] No code files have been changed.
[ ] Other reason

Behavior changed:

[ ] No.
[ ] Yes.

Does this need documentation?

[ ] No.
[ ] Yes.

Check List (For Reviewer who merge this PR)

[ ] Confirm the release note
[ ] Confirm test cases
[ ] Confirm document
[ ] Add branch pick label

run buildall

run buildall

run buildall


Padding needed on iOS, overlapping occurs

Hi,
Padding is needed on iOS. As you can see on the attached image, the lower iOS "Action Bar" overlaps the navigation texts. Maybe some additional padding could be added?

Thanks to your replies to my other comments. Is this something you could look into as well? Love the package but this looks terrible on iOS.

Thanks to your replies to my other comments. Is this something you could look into as well? Love the package but this looks terrible on iOS.

have you tried wrap it in SafeArea widget ?

Thanks, will try it, appreciate the suggestions Were you able to to add the ability to change colors, by any chance?
Sent from my iPhone. Please excuse brief replies.

On Jun 20, 2021, at 18:13, Mohammad Reza Safari @.***> wrote:

Thanks to your replies to my other comments. Is this something you could look into as well? Love the package but this looks terrible on iOS.
have you tried wrap it in SafeArea widget ?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Thanks, will try it, appreciate the suggestions Were you able to to add the ability to change colors, by any chance? Sent from my iPhone. Please excuse brief replies.
…
On Jun 20, 2021, at 18:13, Mohammad Reza Safari @.***> wrote:  Thanks to your replies to my other comments. Is this something you could look into as well? Love the package but this looks terrible on iOS. have you tried wrap it in SafeArea widget ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

no, I smiply drop this library in my project. no RTL support . not resizing for icons. it's so simple. if I just find enough time I will make a fork out of it and fix these issues.

Hi @rgbinnovation @MRezaSafari Sorry for the late reply, due to some unavoidable circumstances I was not able to maintain this package for long days. Now I am back and fixes all issues and released a new version(v2.0.3[https://pub.dev/packages/motion_tab_bar]). I believe most of the issues are fixed now with RTL support, icon resizing, bg color chnages etc.


Is it possible for you to comment each line of the code for pedagogical purposes?

Is it possible for you to comment each line of the code for pedagogical purposes?

What sort of commentary are you looking for? I don't necessarily want to be in the business of writing python tutorials or tensorflow tutorials, but stuff like MCTS or stuff specific to alphago, maybe.

Basically, if you could summarize the job of each function above it and comment lines where variables need to be defined.

A lot of it is already pretty well commented at the top of each file, as to how the whole thing is arranged. Is there something in particular that you're confused by?

i read your code and it confuse me a lot in the go.py,i want to know these functions:is_koish;is_eyeish(i think it's not really the true go rules);pass_move;flip_playerturn(.ko there is always set to None,I'm a little confused about it). and last one:play_move. when I run it with MCTS,sometimes there's a IllegalMove bug like move at (16,16) is illegal. I have study it for a long time ,but still can't resolve. thanks a lot !

@brilee


docs: add @noapprox attribute to number formats

This change adds information about the @noapprox attribute to the number formats section.
Additionaly a link was fixed. pandoc converts dots to dashes and thus clicking on Install via crates.io did nothing. This is now fixed.

The @noapprox attribute can be used to hide the approx. annotation in the output:
> pi to 6 dp
approx. 3.141592
> @noapprox pi to 6 dp
3.141592

Thanks!


Converting ES to JS

Hi guys. Actually I'm converting all react projects to MarkoJS (congratulations, it's amazing).
I found a little problem: browser compatibilities. On react projects, my webpack use babel-preset-es2015/babel-preset-react for convert:
query: {
   presets: ['es2015', 'react'],
},

How to do the same approach in MarkoJS?

Hi @jesse1983
The discussion here might be helpful
https://github.com/marko-js/marko/issues/430
So if you turn on babel to transpile .marko files after marko-loader and transpile .js files, you will have ES6 transpilling support with Marko for the browser.
And if your JS-code for some component is inside component.js, you will still have to use module.exports instead of ES6 export, e.g.
class ComponentClass {
  onInput(input) {
    // ...
  }
}

module.exports = ComponentClass;

You can use the "add-module-exports" Babel plugin to handle convert "export default" to "modules.exports" for you. That way even your components can be ES6.

Take a look my resolution for this issue:
1 - First, I have install Babel Webpack Plugin (https://www.npmjs.com/package/babel-webpack-plugin)
2 - Then, just I added it on webpack.config.js as a PLUGIN e not a LOADER:
/* my webpack.config.js */
plugins: [
   new BabelPlugin({test: /\.js$/, presets: ['es2015']]}
]

I believe that is a approach to solve this case.

@jesse1983 that solution works, but is very slow.
Using babel loader means only individual .js files that change get transpiled.  Using BabelPlugin means the whole output asset(s) gets transpiled


Terraform deploy job

Looks good. Just one question about terraform workspace list Also, it looks like the file is indented with tab. Can you swap them out for two spaces instead just to match the other ones please.


Upgrade Multus to v4.x

Context
v4 is doing a split between thin (as v3) and thick (adding metrics)
Migrate to thick and resolve any issues encountered
Notes for the migration:
Notes gathered when trying to upgrade to 4.0.2, some items may have been resolved since

They moved from a python container image on 3.x to a distroless image on 4.x (cat not available anymore)
Their thick container image uses a slim Debian where we would still have cat  for the startupProbe
Add -thick to the overridden container tag to not have their too loose versioning
--multus-conf-file not available on their thick version, being a flag only for their thin entrypoint

@scoquelin:
Was able to confirm some of those points locally after installing the thick version :
# /usr/src/multus-cni/bin/multus --version
multus: version:(clean), commit:e1a0d2a3fd4f490861ff1872840fdf0a69200747, date:2024-10-15T16:06:23+00:00

# /usr/src/multus-cni/bin/multus-daemon --help
Usage of /usr/src/multus-cni/bin/multus-daemon:
  -config string
        Specify the path to the multus-daemon configuration (default "/etc/cni/net.d/multus.d/daemon-config.json")
  -version
        Show version
# cat /etc/cni/net.d/multus.d/daemon-config.json
{
    "chrootDir": "/hostroot",
    "cniVersion": "0.3.1",
    "logLevel": "verbose",
    "logToStderr": true,
    "cniConfigDir": "/host/etc/cni/net.d",
    "multusAutoconfigDir": "/host/etc/cni/net.d",
    "multusConfigFile": "auto",
    "socketDir": "/host/run/multus/"
}

# whereis cat
cat: /usr/bin/cat

# cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
NAME="Debian GNU/Linux"
VERSION_ID="12"
VERSION="12 (bookworm)"
VERSION_CODENAME=bookworm
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

The yaml definitions for v4.1.3 point to an image that is tagged with snapshot-thick yet the v4.1.3-thick tag is available from here so I guess we have to override it explicitly with kustomize

@scoquelin:
Here is some documentation attempt on how to configure multusMasterCNI with the thick version : https://github.com/k8snetworkplumbingwg/multus-cni/pull/1245

Open issues related to migration

https://github.com/k8snetworkplumbingwg/multus-cni/issues/1346
https://github.com/k8snetworkplumbingwg/multus-cni/issues/1221


hungarian localisation

Any news on this?

Any news?

?

?

?

?

Could you take a look at this?

@crast please take a look at it

could you please answer at least?

I'm aware of this as well as the other Hungarian translation in #262.  I asked a friend who speaks Hungarian to look over both, they said:

the two translations are almost equally valid, and both are lacking a few commas in my opinion. Anyway, I'm leaning a bit towards 262, it's a bit more specific in a few places. But watch out for line 99: the field specifier is missing from the end of the translation. And "not a valid float" is translated into "not a valid fraction", which is bugging me a bit (line 169 in the other translation is precise)

Since the other PR came first and it has an external vote of approval I'm more inclined to merge that one assuming the two things mentioned get fixed.  Could you give yours another look over and compare notes with #262?

Merged #262, so closing this one.  Thanks for your interest though, it's appreciated!  If you want to make further improvements, more PRs are welcome.


Replace CDN links

I replaced the unpkg links with jsDelivr links. jsDelivr works in the same way, but was built specifically for production usage. This should solve problems described in https://github.com/unpkg/unpkg/issues/88.

[clabot:check]

Is jsDelivr better than cdnjs?

cdnjs uses CloudFlare, jsDelivr uses a combination of four providers, including one in China.

@ronkok I think you were the first to suggest unpkg. OK with this change? Sounds good to me.

Quick CDN propagation is unimportant for function-support.html. For it, I'm content with nearly any CDN.
Quick CDN propagation will be very important for index.html if we wish to make its revision part of the standard release script.
When jsDelivr is used, how quickly do users see the updated files?

@ronkok it's < 60 seconds, in most cases there's no delay at all.

@MartinKolarik Fantastic. I think this PR is a great idea.


Add allocateZeroFilled and reallocateBytes APIs for MappedMemory

Add reallocateBytes APIs for MappedMemory to support aligned
memory pool reallocateBytes API.
Add allocateZeroFilled to optimize memory pool allocZeroFilled
implementation
Optimize allocateBytes implementation for non-mmap MappedMemory
implementation.
Results verified by memory allocation microbenchmark.

@xiaoxmeng Meng, there are CI failures and conflicts with main. Would you rebase and look into CI failures?


Dustin bailey

Broke the Client card down into individual components.  Was getting a little big.
Moved some styling out of index.css that was impacting all modal forms.
General styling for screen aware modal.

i think we we're double working on this


🚧  Add practice exercise book store

Hello @SleeplessByte and @junedev,
Could you tell me if I'm in the right direction?
I think I'm only missing on .meta/proof.ci.js, which should be done soon.
Thanks


Add new 'Career mode'

This will eventually replace story mode as the main mode of the game.
Basic flow: Complete 12 levels, pick a path

[ ] The player begins by choosing a start level: 0, 1 or 2
[ ] Based on the player's performance in the level, they advance a certain number of levels
[ ] After each level they're offered 3 more levels to pick from (11, 12, 13)
[x] After 12 levels, the mode is over
[x] When the mode is over, the player is shown a 'You win' screen

(We can make this pretty later, after we iterate on this design a bit. For now, just keep it raw and functional)
Locks: Complete level 19 to unlock checkpoints

[ ] If the player has previously cleared level 19, they have the option of starting again from level 20
[ ] The player cannot skip level 19
[ ] There are similar gates for level 19, 49, 99, 199, 299...

Visual polish:

[ ] Pretty "choose your start level" screen
[ ] Pretty "advance a certain number of levels" screen
[ ] Pretty "choose from 3 more levels" screen
[ ] Pretty "You win" screen
[ ] There is visual feedback during the level for how many levels they will advance
[ ] When the player is offered 3 more levels to pick from, they're shown advancing through the overworld
[ ] After 4 and 8 levels, there is a interlude

Other stuff:

[ ] They can interrupt most levels at any time -- except for 'hardcore levels'
[ ] There is an overarching goal, and earning money helps you reach it: "9 billion served", "$800,000", "our restaurant is so big and fancy!"

We can dress it up later -- but, that is the basic flow of the mode. Let's keep things focused on gameplay over appearance for the start.

Visual feedback during the level could be handled with playing cards; the suit is "footsteps" and the number corresponds to the number of steps they move.

maybe the player walks by wooden signs which count down from 20 to 1
themes:
world 1: simple introductory levels
world 2: learn about boxes (and sometimes no combos)
world 3: learn about combos (and sometimes veggie pieces)
world 4: learn about pickups
world 5: cheese pieces
world 6: review
9:  0    (7k)
24: 0- 3 (5k)
39: 1- 5 (4k)
59: 3- 7 (3k)
79: 5- 8 (2k)
99: 6- 9 (1k)
109: A0-A2 (1d)
124: A1-A4 (2d)
139: A2-A6 (3d)
159: A3-A8 (4d)
179: A4-AA (5d)
199: A5-AD (7d)
200+: FA-FFF
these are all insanely hard rehashes of old levels
after 7 levels, you face a trial from 8d-M based on your performance

This is finished, except for the "overarching goal" but I'm not sure what that will look like, or whether it's necessary.


Added Service Install Functionality

I resolved conflicts at #9
Please let me know if any change required.
Note: My first PR was able to do on close the installation form, if any service installed, they will be selected in the main form that they can be started easily by pressing the Start button. Unfortunately I could not refactor these changes because of async await things. I will do if you want.
Thanks!

I plan to tweak this a little once merged but thank you for your contribution

Thanks @BlythMeister!


Fix incorrect navigation when update Report welcome message after visit by direct link

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/22041
PROPOSAL: https://github.com/Expensify/App/issues/22041#issuecomment-1616422526
Tests

Test case 1: Report welcome message page
Pre-step: Go to any room that you have permission to update welcome message => Go to Room welcome message paste => Copy the URL => Paste to any chat report

Click on the report welcome message link that you just paste
Update the welcome message
Click save button
Verify that it navigations back to the Report setting page

Test case 2: Workspace invite message page
Pre-step: Go to any Workspace, invite any users => On invite message paste => Copy the URL => Paste to any chat report

Click on the WS invite message link that you just paste
Click back arrow in the top bar
Verify that it navigations back to the WS invite member page

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

Offline tests

Not affected by network
QA Steps

Same as Tests

[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 / Chrome
[x] iOS / native
[x] iOS / 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 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] If any non-english text was added/modified, I verified the translation was requested/reviewed 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 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] I verified that if a function's arguments changed that all usages have also been updated correctly
[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] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
[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(themeColors.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 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 author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

https://github.com/Expensify/App/assets/9639873/9b253934-10b5-4f60-8851-3872ba386891
Test ReimbursementAccount - Enabled page
https://github.com/Expensify/App/assets/9639873/648fc110-4900-478d-833d-1829538b37e6

Mobile Web - Chrome

https://github.com/Expensify/App/assets/9639873/f9f3b09c-50db-4aa1-82df-75e30a4b4a97

Mobile Web - Safari

https://github.com/Expensify/App/assets/9639873/76f31df4-db08-4730-9af6-03b91a101a94

Desktop

https://github.com/Expensify/App/assets/9639873/6a8e4cd1-3f2e-49de-9b12-8124b25c0111

iOS

https://github.com/Expensify/App/assets/9639873/5af854ff-6a23-4928-8db0-86e23a15d80b

Android

https://github.com/Expensify/App/assets/9639873/e83e4586-43b6-4bbd-b21c-c543347aa78e

@situchan when I test the "Test case 2". There is a regression bug if we add fallback route, it's loop navigation if we tap on the arrow back on the top bard on native devices
Screen.Recording.2023-08-22.at.05.57.22.mov
I think we can revert the change for this "Test case 2" and leave as it's. What do you think?

Agree, revert this change as it works as expected already
So you fixed 2 pages so far - workspace invite message, welcome message.
Are you sure that no other occurrences throughout the app?

@hoangzinh we should fix all possible cases even though Navigation.goBack is not defined.
HeaderWithBackButton's onBackButtonPress callback returns Navigation.goBack() as default.
i.e.
https://github.com/Expensify/App/issues/22887 was closed in favor of our issue.

Please request review when all feedback is addressed.

@hoangzinh we should fix all possible cases even though Navigation.goBack is not defined. HeaderWithBackButton's onBackButtonPress callback returns Navigation.goBack() as default.

I clearly stated that we should find & fix all occurrences of HeaderWithBackButton.
Btw, I am not able to review this sooner so gonna ask another C+ for help

Still inclined to hold this for now. And see how https://github.com/Expensify/App/issues/24291 goes.

@situchan Updated with latest main. Could you help to review this PR + my comment here #25543 (comment). Thanks

I am good with https://github.com/Expensify/App/pull/25543#issuecomment-1700958653.
WalletStatementPage: https://staging.new.expensify.com/statements/202308

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 / Chrome
[x] iOS / native
[x] iOS / 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 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 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(themeColors.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 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

Web
https://github.com/Expensify/App/assets/108292595/93e1a68a-1408-4f8e-a503-97e43711736e

Mobile Web - Chrome
https://github.com/Expensify/App/assets/108292595/04fd2df2-e083-4044-8e50-5c79834c5ae1

Mobile Web - Safari
https://github.com/Expensify/App/assets/108292595/22d261e2-b1d4-443d-a5b3-3234b37a0e19

Desktop
https://github.com/Expensify/App/assets/108292595/410f158a-8c0d-4c8e-a5a4-01cd90bef83e

iOS
https://github.com/Expensify/App/assets/108292595/796dc461-f691-4475-961f-2fbfb60969c6

Android
https://github.com/Expensify/App/assets/108292595/afbaf124-7528-4d4a-8d45-3ff42be58c7c

: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.

🚀 Deployed to staging by https://github.com/Beamanator in version: 1.3.80-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.80-3 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to staging by https://github.com/Beamanator in version: 1.3.81-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.83-11 🚀

platform
result

🤖 android 🤖
failure ❌

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform
result

🤖 android 🤖
skipped 🚫

🖥 desktop 🖥
skipped 🚫

🍎 iOS 🍎
skipped 🚫

🕸 web 🕸
skipped 🚫

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅


Feature: Add query params to the /feed endpoint

Type of feature
🍕 Feature
Current behavior
Currently, when the OpenSauced chrome extension redirects a user to insights from their PR, nothing is pre-filled.
ref the GIF here - https://github.com/open-sauced/browser-extensions/pull/64
If the /feed endpoint accepted query params like prLink/description:
/feed?prLink=github.com/x/y/pr/55
We could fill in the PR field directly and improve UX.
Suggested solution
Add query params support to /feed endpoint to improve third-party integrations.
This would give users another reason to use the chrome extension too! Happy to look at possible options and implement this.
Additional context
No response
Code of Conduct

[X] I agree to follow this project's Code of Conduct

Contributing Docs

[X] I agree to follow this project's Contribution Docs

Duplicate of https://github.com/open-sauced/insights/issues/1143


Cant use npm run restore

For some reason when using npm run restore
I am getting this error:
Restore platforms is complete

Restoring Plugins

cordova plugin add<EMAIL_ADDRESS>
/Volumes/Data/Stack/projects/wordpress-hybrid-client/node_modules/ionic/node_modules/ionic-app-lib/lib/state.js:543
        throw 'Error executing "' + pluginCommand + '":\n' + output;
        ^
Error executing "cordova plugin add<EMAIL_ADDRESS>Fetching plugin<EMAIL_ADDRESS>via npm
Plugin "cordova-plugin-whitelist" already installed on android.
Plugin "cordova-plugin-whitelist" already installed on ios.
Error during processing of action! Attempting to revert...
Error: Uh oh!
Invalid Version: undefined

node, npm and wphc versions please

Node: 2.14.12
Npm: v4.2.6
WPHC: v2.0.0-beta06
on OS X El Capitan

ok, try upgrading node >= 4

Okay, updated node (v4.2.6) but i am still seeing the same error...

rm -rf node_modules && npm install

Still not working for some reason...

what about if you run the command
cordova plugin add<EMAIL_ADDRESS>?

$ cordova plugin add<EMAIL_ADDRESS>Fetching plugin<EMAIL_ADDRESS>via npm
Plugin "cordova-plugin-whitelist" already installed on android.
Plugin "cordova-plugin-whitelist" already installed on ios.

Your npm version looks weird. Latest is 3.x

Well for some reason it just worked after the cordova plugin add<EMAIL_ADDRESS>Thanks @shprink

good to know :) #magic


Home Page

Home Page
User Story
AS A developer who writes about tech
I WANT a CMS-style blog site
SO THAT I can publish articles, blog posts, and my thoughts and opinions
Acceptance Criteria
GIVEN a CMS-style blog site
WHEN I click on the homepage option
THEN I am taken to the homepage

Application now meets all acceptance criteria


Unable to schedule cluster-autoscaler to worker nodes

cluster-autoscaler(CA) is scheduled to controller nodes by default.
It is designed to be configured to be scheduled to worker nodes by setting worker.nodePools[].clusterAutoscalerSupport.enabled to true in cluster.yaml.
However, my manual testing revealed that the setting doesn't work at all.
More concretely, a kube-aws.coreos.com/cluster-autoscaler-supported node label which is expected to be added to nodes when the support is enabled is just missing from the nodes.

The node label was intended to be added from this part of kube-aws code base:

func (c WorkerDeploymentSettings) NodeLabels() model.NodeLabels {
	labels := c.Experimental.NodeLabels
	if c.ClusterAutoscalerSupport.Enabled {
		labels["kube-aws.coreos.com/cluster-autoscaler-supported"] = "true"
	}
	return labels
}

However, this might not work as this func is not called from anywhere.


drivers/mfd: it8801: Implement ITE IT8801 mfd drivers

This PR Implements the ITE IT8801 IO expander multi-function device driver. The IO expander provides GPIO, PWM, and keyboard functions via the I2C bus.

Just an quick initial, pass, noticed some comments of mine overlaps with Keith.
Hey is there a board where you could add this? I'm a bit confused by the dtsi file you have in one of the commits, would be nice to have build testing in CI as well.

I have created a test program to connect with it8xxx2_evb as shown in the link

I have created a test program to connect with it8xxx2_evb as shown in the link

Could you make some equivalent ones in tests/drivers/build_all/input/app.overlay, tests/drivers/build_all/pwm/boards/native_sim.overlay and tests/drivers/build_all/gpio/app.overlay?

Rebased.

I have created a test program to connect with it8xxx2_evb as shown in the link

Could you make some equivalent ones in tests/drivers/build_all/input/app.overlay, tests/drivers/build_all/pwm/boards/native_sim.overlay and tests/drivers/build_all/gpio/app.overlay?

I'm going to create this. Is this part also included in this PR?

I'm going to create this. Is this part also included in this PR?

That would be ideal yeah, that way the code gets built from the go.

I'm going to create this. Is this part also included in this PR?

That would be ideal yeah, that way the code gets built from the go.

Done.

Rebase and resolve conflicting files.

Hi @fabiobaltieri, @keith-zephyr, If there’s any further feedback on the updates I made. Thanks!

Rebased.

cool, couple missing static qualifiers, rest is good, great job, Keith is out for the next couple of weeks I think it's fine to wait for him?

OKey, thank you for your feedback!


Reorganise the intro pages

I'm making changes via PRs for clarity while we merge our example notebooks etc


Add wrapper support for diff and lint

Added wrapper support for core helm lint and diff plugin helm diff commands, which both take -f argument.
Also used extglob to prevent repeating the supported command list twice.

#38 already exists for that but you added a nifty extglob support so I'll close mine :)

@helgi Yeah, I also saw your PR, I created the PR a couple of weeks ago, but I did it against my fork by mistake. :-)

Merged, thanks.


Joel Grus: Learning Data Science Using Functional Python | PyData Seattle 2015

https://www.youtube.com/watch?v=ThS4juptJjQ&list=WL&index=50
Contents
0:00  -  Introduction
0:45  -  What is functional programming?
2:57  -  Iterators
3:55  -  Generators/Generator comprehensions
6:10  -  itertools
12:30 -  Fibonacci numbers example
15:56 -  Prime numbers example
16:57 -  K-means clustering
26:10 -  Aside: Matplotlib animation for K-means
27:40 -  Gradient Descent
35:17 -  Linear Regression for Stochastic Gradient Descent
37:30 -  End of lecture and Questions
let me know if i can improve something.

Great, thank you!


New cop rule:  IrreversibleMigration

Should resolve this issue: https://github.com/rubocop/rubocop-sequel/issues/30
According to the Sequel documentation, only certain methods should be placed inside a change block in a migration file. Other methods could make the migration irreversible.
This new cop will warn the user under the following conditions inside a change block:

They use a method that's not listed in the documentation
They use the method #add_primary_key with an array argument instead of a symbol

This introduced an issue where change blocks outside of Sequel migrations are also being detected. See #44 for context.

The IrreversibleMigration rule disallows using primary_key and String for create_table in a change block. However, the Sequel documentation example for how to use change in lieu of up/down blocks uses primary_key and String methods for a create_table in a change block. I'm a bit confused here as to why these are not allowed by IrreversibleMigration? Would the example documentation also fail this cop?

@soumyaray Thanks for bringing this to my attention, the cop should be ignoring all methods within a create_table block. I will work on a fix for this

@soumyaray Thanks for bringing this to my attention, the cop should be ignoring all methods within a create_table block. I will work on a fix for this

Thank you so much for your quick handling of this -- really appreciate the work on this gem.


Cosy Pose Eval Code - n_top number

In cosy pose eval code, when we set n_top to 1, I saw ADD and other mAP values can be higher than 1.  I think that code during matching uses all object available in the scene but during division it only uses n_gt_valid and so, results are being higher than 1. This is the cases where more than one object for a class is in the scene. Is this a bug?

Can you be a little be more specific on which results you are looking at so that I can take a look? (datasets, models, metrics)

I was talking for a custom model and custom dataset but is similar to Tless format. We were looking ADD-0.1 metric


Correction for raw affiliation INRA-UMR IATE, Montpellier Cedex 2, France<EMAIL_ADDRESS>

Correction needed for raw affiliation INRA-UMR IATE, Montpellier Cedex 2, France<EMAIL_ADDRESS>raw_affiliation_name: INRA-UMR IATE, Montpellier Cedex 2, France<EMAIL_ADDRESS>new_rors: 03rnk6m14;0000n5x09
previous_rors: 03rnk6m14
works_examples: W2074774419
contact: 60fe996cc895f6d0831ab05896cfb25a:f8360bf7a165d4f9eace19185039eaa51d6b @ inrae.fr

This issue was accepted and ingested by the OpenAlex team on 2024-10-09. The new affiliations should be visible within the next 7 days.


feat: use agent message contents to display messages

Description
We now have the new AgentMessageContent table to store all the raw contents generated by the assistant at each step of the agent message.
With this commit, we'll start using these raw contents to re-construct content (what is rendered in the body of the message) and chainOfThoughts (what is rendered in AssistantThoughts).
There is one user-facing change here: content emitted before tools use will not be "teleported" inside of the assistant thoughts when the tools use occur -- it will remain in the content, where it should be.
next steps:
once we are fully convinced everything works great, we'll delete the content and chainOfThoughts columns from the agent message model. We should also be able to get rid of the agent_chain_of_thoughts event, since its main purpose is to store data in the chainOfThoughts column.
Risk
Obviously in the critical path. Well tested. We can also test it on front-edge
Deploy Plan
N/A

What are the next steps? I presume we want to clean-up the agent message model a bit?

As mentioned in OP, precisely that yes 👍


ui-keypress object literal

It is strange to write
<textarea ui-keypress="{
    13: 'keypressCallback($event)'
}">

Should simply be
<textarea ui-keypress="{
    13: keypressCallback($event)
}">

'keypressCallback($event)' makes it look like a string literal while in fact it is not.

UI.Utils modules was split in individuals repositories. If still valid, please consider re-submitting the issue on its dedicated issue tracker.
See the README for details.


The class attribute doesn't work in addElement

when adding an element via $kanban.addElement(), the classes specified in the class attribute are not added as classes in the element itself.
Also could you please update the npm package (https://www.npmjs.com/package/jkanban), with the new code ?

fix in #69


Make genesis ledger sequence configurable

@bachase, @mellery451 Thanks for the feedback. I pushed a commit addressing comments. I'll work on extending the database unit test to add coverage for this new feature.

@bachase @mellery451 Unit test added.

I'm getting some unit test failures
ripple.NodeStore.Database NodeStore backend 'memory'
#4 failed: Database_test.cpp(178)
ripple.NodeStore.Database NodeStore backend 'nudb'
#1 failed: Database_test.cpp(121)
#6 failed: unhandled exception: genesis_seq set more than once
285ms, 1 suite, 2 cases, 12 tests total, 3 failures

Fixed test faile issue.

tests look good now, getting one warning that looks easy to fix:
src/test/nodestore/Database_test.cpp|207 col 22 warning| unused variable 'i' [-Wunused-variable]
                 auto i = std::strcmp(e.what(),

@mellery451 Yikes, that line with the warning ws left over debug code. Thanks.

👍

Jenkins Build Summary
Built from this commit
Built at 20180301 - 06:41:40
Test Results

Build Type
Result
Status

Codecov Report

Merging #2410 into develop will decrease coverage by <.01%.
The diff coverage is 34.54%.

@@             Coverage Diff             @@
##           develop    #2410      +/-   ##
===========================================
- Coverage    70.32%   70.31%   -0.01%
===========================================
  Files          702      702
  Lines        53429    53439      +10
===========================================
+ Hits         37573    37578       +5
- Misses       15856    15861       +5

Impacted Files
Coverage Δ

src/ripple/nodestore/impl/DatabaseRotatingImp.h
50% <ø> (ø)
:arrow_up:

src/ripple/nodestore/impl/Shard.cpp
0.39% <0%> (+0.01%)
:arrow_up:

src/ripple/nodestore/impl/DatabaseShardImp.cpp
0% <0%> (ø)
:arrow_up:

src/ripple/nodestore/DatabaseShard.h
0% <0%> (ø)
:arrow_up:

src/ripple/app/ledger/impl/LedgerMaster.cpp
45.8% <0%> (-0.06%)
:arrow_down:

src/ripple/overlay/impl/PeerImp.cpp
0% <0%> (ø)
:arrow_up:

src/ripple/nodestore/impl/ManagerImp.cpp
82.97% <100%> (ø)
:arrow_up:

src/ripple/nodestore/Database.h
100% <100%> (ø)
:arrow_up:

src/ripple/nodestore/impl/DatabaseRotatingImp.cpp
39.56% <100%> (ø)
:arrow_up:

src/ripple/app/misc/SHAMapStoreImp.cpp
75.78% <100%> (ø)
:arrow_up:

... and 6 more

Continue to review full report at Codecov.

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

@nbougalis Thanks for the feedback.  It also fixes an old bug where we don't check to make sure we don't route a ledger request back to the peer we got it from. I have reviewed his changes and given them the green light.

Not sure if this helps, but to me, the  genesis ledger is the ledger that has no parent and is the initial/first ledger. Then earliest ledger is the ledger closest to the genesis that we believe is available.

I've added my approval, but I didn't review the whole PR. This just acknowledges that my issue was resolved.

In 1.0.0-b3 as 0b18b36186797152f49d0dd4e388b8ad98c04820
and 5db5e3114014173113324ff885a5859053fd0eb4


Configure web UI background as a visual reminder of environment

Relayed user feature request from NYC Container Days...
"We have 10 different environments, from devtest to UAT to prod, and we would like a visual reminder of which environment we are logged into to prevent accidentally mucking up something in the wrong environment. I think the easiest and most difficult to miss reminder could come via the ability to configure an environment to have a specific background color and/or background image."
Customer is considering the idea of a browser plugin with CSS mods a la Greasemonkey etc but would prefer that we provide the functionality natively.

This would be great, I have been asked several times about "branding" environments for specific teams.

Yes, we need this as well, though perhaps coloring the navigation bar is enough.

With the release of Rancher 2.0, development on v1.6 is only limited to critical bug fixes and security patches.


Transaction id is too long

The transaction id is too long, it breaks in several lines. It would be better to display just the six (or four) beginning and ending characters of the id with dots between. For example 0x800439...be3e28.
Also the # is not needed because the identifiers are not sequential, but random and in the Ethereum ecosystem people are used to the 0x... notation.

Yup we need to check in with David to see what the intention is for the transaction #. His design shows #001 so we're not sure what he wants. I left a comment in the code so we don't forget 😉

So the idea here was to have an unique identifier for a transaction. I didn't plan to use the actual transaction number, but rather a simple sequential number. Keep in mind that in the case of a request transaction we will not have an actual transaction number upfront, but we will still need this unique identifier (to help people distinguish between transactions if they have multiple active ones). In that case the "transaction" is the Payggy transaction, not necessarily and actual payment. Think also about a split payment scenario (for later) one Payggy transaction might have multiple payments associated to it (each with an actual transaction number). Does that make sense?

We had a conversation with @vojtechsimetka and @sugarpusher yesterday about this. @vojtechsimetka mentioned that we already have an "Instance ID" for Waku Objects instance, so we agreed to use the first 4 characters of that "Instance ID"

I updated both the Payggy and SafeMeet sketch files to reflect this change.
Note that I also remove that field from from the summary screen since this is not editable


Download Pre-trained newer version models

How to download Pre-trained newer version(v0.02 onwards) models, trained on 1 million datasets?

any place where we can dl the earliest model? or shuld we gather training data and train it ourselves?

is it ok to collect the data for this project and post it in here ?

Does

anyone have links to any of the trained models?
Please do share.

Can Someone Please Share Their Trained Models I Have a Cpu And I can not train it for a month but I want to see how it works after models if anyone is kind enough to do this favor


⚠ Bofrost.de has degraded performance

In 07b3972, Bofrost.de (https://www.bofrost.de) experienced degraded performance:

HTTP code: 200
Response time: 960 ms

Resolved: Bofrost.de performance has improved in 25e447c after 6 minutes.


Getters and Setters for Parameterized Access Policies

After completing the foundation of Parameterized Access Policies, we need to add getter/setter endpoints for a user's ProjectMembership to allow admin-level clients to assign these policies to a user programatically.

@rahul1 - I believe that this is resolved by #1602 - can you please confirm (or deny and close)?

Closed by #1602


Please build and publish this package, instead of including an not-compiled .vue.

This way this package can be used in non-webpack apps.

@laosb You can try to use, compile for the first time, do not know if it can run.
https://github.com/surmon-china/vue-quill-editor/blob/master/dist/index.js

Will try in Meteor when time is with me. Thx!

The problem is temporarily closed, if the problem is reproduced, please reopen it.


Fix Empty Z file

For some reason when a tf object created in aurora is set to write a z file the elements are missing. There is also no metadata in the tf object.

Codecov Report
Base: 79.20% // Head: 79.01% // Decreases project coverage by -0.19% :warning:

Coverage data is based on head (7c8701d) compared to base (8f244b8).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #109      +/-   ##
==========================================
- Coverage   79.20%   79.01%   -0.20%
==========================================
  Files         216      212       -4
  Lines       14184    14116      -68
==========================================
- Hits        11235    11154      -81
- Misses       2949     2962      +13

Impacted Files
Coverage Δ

mt_metadata/transfer_functions/io/readwrite.py
36.00% <ø> (ø)

mt_metadata/transfer_functions/io/zfiles/zmm.py
62.45% <0.00%> (-1.70%)
:arrow_down:

mt_metadata/__init__.py

mt_metadata/timeseries/stationxml/__init__.py

mt_metadata/timeseries/filters/__init__.py

mt_metadata/timeseries/__init__.py

mt_metadata/utils/mt_logger.py
72.72% <0.00%> (+1.08%)
:arrow_up:

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.


Add gradle wrapper validation GitHub Action

It will be helpful to validate gradle-wrapper.jar integrity.

From action page, we can see following log:
✓ Found known Gradle Wrapper JAR files:
  70239e6ca1f0d5e3b2808ef6d82390cf9ad58d3a3a0d271677a51d1b89475857 src/conformance/gradle/wrapper/gradle-wrapper.jar
  70239e6ca1f0d5e3b2808ef6d82390cf9ad58d3a3a0d271677a51d1b89475857 src/loader/gradle/wrapper/gradle-wrapper.jar

All gradle-wrapper.jar in this repository are validated by official GitHub Action. If other contributors update gradle-wrapper.jar at future commits, and this Action passed, it's safe to merge commits to update gradle-wrapper.jar.

cc /@rpavlik . FYI, Looks like GitHub Action to build conformance Android apk works fine.

Fancy, is this often needed? Guess it helps when updating the wrapper.

The file needs copyright and license headers. Apparently I didn't set up the reuse check here yet

Done.

The file needs copyright and license headers. Apparently I didn't set up the reuse check here yet

It's one of best practice for Android project on GitHub. It will help gradle-wrapper.jar's integrity if contribution is made by outer contributor, for example like me to avoid file injection dangers.

It's official page for this action: https://github.com/marketplace/actions/gradle-wrapper-validation.

Oh very good idea, I'll have to do this elsewhere too.

Oh very good idea, I'll have to do this elsewhere too.

@rpavlik could we merge it firstly? Thanks.

Oops, sorry, didn't realize I missed it.

Oops, sorry, didn't realize I missed it.

Thanks.


Hack Club SEO

Thanks to AlexB for reminding us that our SEO is not very good:
https://s3.amazonaws.com/f.cl.ly/items/0K2N0Z0h3C0U0j0U3830/Image 2015-11-30 at 12.20.50 PM.png?v=b7663941

I've added hackclub.io to Google's webmaster tools and have requested it to be re-indexed. Hopefully that'll help.


Update rules_go

Took a quick shot at updating rules_go, since the old version being used overrides Bazel platforms// with a much older version that has some problems.
(For more, see https://github.com/bazelbuild/rules_go/issues/3071)
Might be worth setting up Renovate (or similar) like some of the other bazel repos to keep dependencies up to date by default?
(For more, see https://github.com/renovatebot/renovate)

@nicolasnoble

Hmm, looks like the tests are taking a long while to return. Is everything okay there?
And it looks like mergable is new--and might need a label applied. Lmk if you'd like me to do something there.

Just pushed another commit. Do I need to ask for a label to be added to rerun CI?

👋🏻 Okay, so the test failures are:

For distribtest: Looks like rules_go has updated its minimum Bazel version to 4.2.1. Up to you guys on how you want to handle things when an (optional?) dependency upgrades its minimum version.
[For macOS: VM timeout.]

I think each of these would probably need gRPC folks to make decisions, so I'm gonna ask to hand this over to you guys and tap out myself. [Also because this is getting a bit snarled, and we don't actually need gRPC at the moment--just an ex-Googler trying to help out after happening to notice the propagated issue.]

Will probably close/delete this in a week or two if I don't hear anything.
[Just trying to help fix things; so if not of interest, that's fine.]

Hey, all! One last, friendly bump :)

Bummer. Sad only bc I'd wanted to help.

The test log says Bazel distribtest failed: Failed to build with bazel versions 3.7.2 4.0.0
see log here:
https://source.cloud.google.com/results/invocations/a01bddf0-1593-48cc-b4ab-d068e79b590b/log

I think the macOS C/C++ bazel test is unrelated (we can ignore).
The bazel distribtests is a problem. Here's our bazel support policy:
https://github.com/grpc/grpc/blob/master/doc/bazel_support.md
(unless we can make things support the versions listed in https://github.com/grpc/grpc/blob/master/doc/bazel_support.md#supported-versions, I'm afraid this PR is a no-go).

Hey! Good to get a reply. @jtattermusch, those were exactly the conclusions that motivated the questions above, esp whether we should do a graceful fallback to an older rules_go when needed to continue supporting older Bazel versions.
Regardless, looks like you all just landed an equivalent upgrade in #29219, without worrying about backwards compatibility 🤷🏻, (same reviewers, too). So I'll close this down.
Respectfully and kindly, I should say that this wasn't the most fun contributing process I've gone through on GitHub (or with gRPC). Anything I should do to make it more worthwhile next time?
And finally, friendly heads that rules_go and Renovate (see original comment) have some great tools for helping automate the work of keeping Bazel dependencies up to date. Maybe those would be useful to you guys, too!
All the best,
Chris

Hey! Good to get a reply. @jtattermusch, those were exactly the conclusions that motivated the questions above, esp whether we should do a graceful fallback to an older rules_go when needed to continue supporting older Bazel versions.
Regardless, looks like you all just landed an equivalent upgrade in #29219, without worrying about backwards compatibility 🤷🏻, (same reviewers, too). So I'll close this down.

But it looked that for the other PR, the tests have passed, which seems to be a quite important difference?

Respectfully and kindly, I should say that this wasn't the most fun contributing process I've gone through on GitHub (or with gRPC). Anything I should do to make it more worthwhile next time?

Understood. Note that I wasn't the assigned reviewer, I was just trying to help out when things seemed to get stuck.
I think you did well with your contribution, unfortunately the volume of incoming pull requests isn't small and it's not the only thing our team does, so it's not always easy to respond/review in timely manner. I acknowledge this is less than ideal and we are trying to improve, but it's not easy since there's some inherent complexity to every PR (especially when it touches the build) and we can't simply accept contributions without giving them a proper though first.

And finally, friendly heads that rules_go and Renovate (see original comment) have some great tools for helping automate the work of keeping Bazel dependencies up to date. Maybe those would be useful to you guys, too!
All the best, Chris (ex-Googler)

Totally! Thanks for the gift of feedback and for picking it up when you saw it stuck. Super appreciate that. And I also understand the challenges of keeping up as a maintainer, being one myself. Just trying to learn if there's something I should do to engage better.
Having some trouble looking back at test logs (they're now 401'ing), but heads that I think the rules_go version in #29219 (0.31 instead of 0.30) also (at least nominally) requires Bazel 4.2.1 (from release notes of previous), so it seems likely that the dependencies-have-higher-minimum-requirements issue might resurface. Hence the original questions. Totally fair about the tests, though :)
Cheers!


upgrade gatsby

boom :boom:

conflicts resolved
no new security vulns
tests passing

I've labelled this blocker because I'd quite like to merge this before dependabot pops up on Friday and we get a bunch of merge conflicts on package-lock.json, although if I do need to do it again, I can.

Messages

:book:
:sparkles: Thanks for your contribution to Shields, @chris48s!

  Generated by :no_entry_sign: dangerJS against 9eac07fd581ebbb5a072743e4bc9b637b670714d

Also, I think I get the gist of the skipLibCheck tsconfig option, but would you mind giving me a quick recap of the impact and implications for us here?

The issue we've got with gatsby/express-graphql/graphql is basically around type checking/incompatibility with tsc. Using skipLibCheck disables all type checking in .d.ts files. See https://bobbyhadz.com/blog/typescript-disable-type-checking-node-modules . There is a tradeoff here. We do lose some type safety not only within node_modules but also to a lesser extent within our own codebase: It does also disable https://github.com/badges/shields/blob/master/frontend/types/assets.d.ts and https://github.com/badges/shields/blob/master/frontend/types/mapbox__react-click-to-select/index.d.ts .

Thanks for that. Agreed the tradeoffs are worth it here, we've been in dependency upgrade hell for too long


Submission start - Create and autofill the peak group conflicts sheet

Summary Change Description
Create and autofill the peak group conflicts sheet.
Unfortunately, while this works, it triples the amount of time it takes to process peak annotation files, but I decided to check in this working version and then try and optimize it, perhaps using pandas operations on the data instead of loops...
Details:

Added the PeakGroupConflicts loader and sheet to the BuildSubmissionView.
Added the ability to autofill 1-cell dropdowns from the autofill_dict by setting the autofill value to the dict needed by xlsxwriter's data_validation method.  When autofill is added, these dicts are saved in a validation_autofill_dict.  When that dict is processed, it supplies the dropdown dict to xlsxwriter.data_validation with the sheet, column, and row of where it should go.
Added a search for multiple representations by first recording all representations, adding representations that already exist in the DB, grouping them by file set, then grouping the file sets by common samples.
Added the ability to hide columns by creating a new optional class attribute to TableLoader, with associated methods, and added the common samples column to PeakGroupConflicts as a hidden column.  A new corresponding method in submission.py was added and called from the add_annotations method.
Changed the display order of the sheets to represent the order in which sheets should be filled out: Study, Tracers, Infusates, Animals, Samples, Sequences, Files, Details, Conflicts...

Affected Issues/Pull Requests

Resolves #1095
Resolves #1092
Merges into PR #1209

Review Notes

See comments in-line.
Checklist

This pull request will be merged once the following requirements are met.  The
author and/or reviewers should uncheck any unmet requirements:

Review requirements

Minimum approvals: 1
No changes requested
All blocking issues resolved by reviewers
Specific reviewers: @add_username_here

Review period: 2 days

Associated issue/pull request requirements:

[x] All requirements in affected issues marked "resolved" are satisfied
[x] All required pull requests are merged (or none)

Basic requirements

[x] All linters pass
[x] All tests pass
[x] All conflicts resolved

Overhead requirements

[x] New/changed method tests implemented/updated (or no change)
[ ] Updated Unreleased section of changelog.md (or no change)
[x] Migrations created & committed (or no change)

So regarding my speed concerns (using all of the peak annotation files from the water study), my laptop at home (which tends to run really slowly), can take anywhere from 2 minutes to 6 minutes to get to the template download.  My iMac at work (even though it's older) tends to be faster.  I just ran it on the water data and timed each portion of the algorithm. In total, it took a minute 40.  The breakdown is:

Reading the files (which has not changed): 31s
Compiling all the existing autofill data (which hasn't really changed significantly): 10s
NEW PART: Searching for multiple representations: 52s
Building the excel file: 1s

So it was close to 1m (42s) before the multiple representations.  The algorithm I used is described in the design portion of the issue.
I may be able to prune the existing search space a good bit to make it more efficient.
I just thought of a decent way to skip the deeper combinatoric portion of the algorithm.  The part that probably takes the most time is the part that allows files to not 100% overlap samples.  Let me explain the allowance for sample subsets with a concrete example...
Say we have 2 peak annotation files that each have Lysine (positive and negative scans).

accucor_pos_high.xlsx has 20 samples
accucor_neg.xlsx has 25 samples (20 samples in common with file 1 - which I believe may be unusual)

If the user selects accucor_pos.xlsx for Lysine, we want to skip the Lysine PeakGroups in accucor_neg.xlsx, but only for the common 20 samples.
If it was just 2 files, this wouldn't be too hard, but what if we have a 3rd file?:

accucor_pos_low.xlsx has 5 samples in common with accucor_neg.xlsx

And the user needs to select (let's say) accucor_neg.xlsx for those 5 samples.
It's this scenario that takes so much time: finding what files overlap with some subset of samples so that we can offer them all the possible choices.
I suspect that this may be an uncommon scenario, but I think it can have a negative impact on data integrity if we do not support all the cases.
However, my idea is to check if the sample overlap is 100%.  If it is, we may be able to skip at least 1 of the extra loops.
I'm going to check dev and see if the running time on this largest case is any better.  If it's under 1m, maybe all we should do is just bump up the apache timeout...

OK, from the testing, it actually appears that it's the database queries that takes that entire 52s.  I can completely skip that in the current case with a quick check just to see if any of the samples already exist.  That may sufficiently solve the speed problem.  Going to give it a shot.  I may also be able to make the query more efficient too, so I might try that before I skip the DB query, just so I have something to compare against.

Ha!  I made the added running time negligible!  52s down to nearly 0s added!  Commit/push pending...