Conversation
Signed-off-by: Keran Yang <yangkr920208@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
=======================================
Coverage ? 58.76%
Complexity ? 476
=======================================
Files ? 151
Lines ? 3390
Branches ? 231
=======================================
Hits ? 1992
Misses ? 1231
Partials ? 167 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This reverts commit 01ee28e.
Signed-off-by: Keran Yang <yangkr920208@gmail.com>
| } | ||
| @Test | ||
| @Order(1) | ||
| public void testMapServerInvocation() { |
There was a problem hiding this comment.
This file is not changed. Just IntelliJ re-format.
| public void testSimpleSink() { | ||
| int datumCount = 10; | ||
| SimpleSink simpleSink = new SimpleSink(); | ||
| @Test |
There was a problem hiding this comment.
This file is not changed. Just IntelliJ re-format.
| public class Response { | ||
| private final String id; | ||
| private final boolean success; | ||
| private final Boolean success; |
There was a problem hiding this comment.
Change it back because it's backward incompatible. If we ever decide to have a backward in-compatible release, we can make this change. Tracking it here.
|
@yhl25 could you please review? Thanks! |
| distribution: 'temurin' | ||
| - name: Build with Maven, run unit tests and the coverage check | ||
| run: mvn clean install | ||
| - name: Build Examples |
There was a problem hiding this comment.
Can we avoid building docker images? The executions inside the examples pom will build the docker images, which can take longer.
There was a problem hiding this comment.
I added it because I want to make sure there is no issue building the images so that it's more unlikely that the push image action fails. The recent build took 39 seconds.
| import io.numaproj.numaflow.sinker.ResponseList; | ||
| import io.numaproj.numaflow.sinker.SinkerTestKit; | ||
| import io.numaproj.numaflow.sourcetransformer.SourceTransformerTestKit; | ||
| import java.time.Instant; |
There was a problem hiding this comment.
Why is it reformatting? Editor config is not used?
There was a problem hiding this comment.
Thank you @yhl25 ! I tested a bit on my local. It was reformatting because 1. my IDE wasn't configured to respect editor config, and 2. some of the files in the examples folder were not formatted following editor config.
I configured my IDE to respect and also copied the .editorconfig file to examples folder, just so if future developers create a project using examples folder as root, the editor config is still respected by their IDE.
Signed-off-by: Keran Yang <yangkr920208@gmail.com>
getSuccess()toisSuccess. An example was broken because of that.Issue was detected late during the example image build push. This code change adds the building of examples to the CI so that if an SDK change breaks any examples, the issue can be caught during CI instead of after merging.
successfromBooleantobooleanis backward-incompatible to users, who use the getter in udsink. I scanned Intuit repo and saw we do have existing users using the getter. I am reverting theBoolean to booleanchange for now to ensure backward compatiblitity.