Bump outdated deps #119
Bump outdated deps #119
Conversation
SDK Examples Unit Test Results16 files 16 suites 2m 44s ⏱️ For more details on these failures, see this check. Results for commit a8329ee. ♻️ This comment has been updated with latest results. |
Common Integration Test Results0 files 0 suites 0s ⏱️ Results for commit a8329ee. ♻️ This comment has been updated with latest results. |
WalkthroughThe changes in this pull request involve updates to various configuration and Kotlin files across the project. Key modifications include the addition of a new Ktlint rule in the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (31)
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getExecutionData/GetExecutionDataAccessAPIConnector.kt (1)
8-12: LGTM: Method refactored to use Kotlin idiomsThe refactoring of
getExecutionDataByBlockIdto use single-expression function syntax improves conciseness and readability. The logic remains correct, handling both success and error cases appropriately.Consider enhancing error handling:
Instead of throwing an exception in the error case, you might want to return aResulttype to allow callers to handle errors more flexibly. For example:fun getExecutionDataByBlockId(blockId: FlowId): Result<FlowExecutionResult> = when (val response = accessAPI.getExecutionResultByBlockId(blockId)) { is FlowAccessApi.AccessApiCallResponse.Success -> Result.success(response.data) is FlowAccessApi.AccessApiCallResponse.Error -> Result.failure(Exception(response.message, response.throwable)) }This approach would allow callers to handle errors without necessarily using try-catch blocks.
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getCollection/GetCollectionAccessAPIConnector.kt (1)
9-12: LGTM: Improved method conciseness with a minor suggestionThe refactoring of
getCollectionByIdto a single-expression function improves conciseness and readability while maintaining the original functionality. This change aligns well with Kotlin's idiomatic style.A minor suggestion for improvement:
Consider wrapping the exception throwing in a separate function to enhance readability and reusability. For example:
private fun handleError(response: AccessApiCallResponse.Error): Nothing = throw Exception(response.message, response.throwable) fun getCollectionById(collectionId: FlowId): FlowCollection = when (val response = accessAPI.getCollectionById(collectionId)) { is AccessApiCallResponse.Success -> response.data is AccessApiCallResponse.Error -> handleError(response) }This approach would make the main function even more concise and allow for centralized error handling if needed in other parts of the class.
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/ExamplesUtils.kt (1)
18-19: LGTM! Consider adding an inline modifier for performance.The refactoring of
ByteArray.toUnsignedByteArrayto use expression body syntax improves code readability. As this is a small, single-expression function, you might consider adding theinlinemodifier to potentially improve performance for frequent calls.Consider applying this minor change:
-fun ByteArray.toUnsignedByteArray(): ByteArray = +inline fun ByteArray.toUnsignedByteArray(): ByteArray = this.map { (it.toInt() and 0xFF).toByte() }.toByteArray()kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getAccount/GetAccountAccessAPIConnector.kt (1)
Line range hint
21-24: Consider refactoringgetAccountBalancefor consistency.While the refactoring of
getAccountAtLatestBlockandgetAccountAtBlockHeightimproves code readability, thegetAccountBalancemethod remains in its original form. For consistency across the class, consider refactoring this method to use expression body syntax as well.Here's a suggested refactoring:
fun getAccountBalance(address: FlowAddress): BigDecimal = getAccountAtLatestBlock(address).balanceThis change would make the entire class consistent in its use of expression body syntax for simple methods.
sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/number/JsonCadenceBuilderUInt8NumberFieldTest.kt (2)
30-32: Improved readability, consider adding an error message assertion.The refactoring of the assertion into multiple lines improves readability. Good job on maintaining the test's logic while enhancing its clarity.
Consider adding an assertion for the error message to make the test more robust:
Assertions .assertThatThrownBy { UInt8NumberField("invalidValue").decodeToAny() } .isInstanceOf(NumberFormatException::class.java) .hasMessageContaining("For input string: \"invalidValue\"")This addition would ensure that not only the correct exception type is thrown but also that the error message is as expected.
Line range hint
1-33: Consider standardizing assertion style for consistency.The test coverage is comprehensive, addressing various scenarios for the
UInt8NumberFieldclass. Well done on the thorough testing approach.For improved consistency, consider standardizing the assertion style throughout the test class. Currently, there's a mix of JUnit's
assertEqualsand AssertJ'sassertThatThrownBy. Choosing one style (preferably AssertJ for its fluent API and rich assertion library) could enhance readability and maintainability.Example of using AssertJ for all assertions:
import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy // ... @Test fun `Test creating UInt8NumberField with valid value`() { val uint8Field = UInt8NumberField("42") assertThat(uint8Field.value).isEqualTo("42") } @Test fun `Test decoding of UInt8NumberField with maximum value`() { val uint8Field = UInt8NumberField("255") assertThat(uint8Field.decodeToAny()).isEqualTo(255U) } // ... (apply similar changes to other test methods)This change would make the tests more consistent and potentially easier to read and maintain.
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getBlock/GetBlockAccessAPIConnector.kt (1)
Line range hint
7-14: Consider refactoringgetLatestSealedBlockfor consistency.While the changes to
getBlockByIDandgetBlockByHeightare good improvements, consider applying the same expression body syntax refactoring togetLatestSealedBlock. This would ensure consistency across all methods in the class and further enhance readability.Here's a suggested refactoring for
getLatestSealedBlock:fun getLatestSealedBlock(): FlowBlock = when (val response = accessAPI.getLatestBlock(isSealed = true)) { is FlowAccessApi.AccessApiCallResponse.Success -> response.data is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable) }sdk/src/main/kotlin/org/onflow/flow/sdk/extensions.kt (1)
Line range hint
1-50: Consider applying consistent formatting to other functions.While the changes in this PR are limited to the
LocalDateTime.asTimestamp()function, it might be beneficial to apply similar formatting improvements to other functions in this file for consistency. This could enhance overall readability and maintainability of the codebase.sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/number/JsonCadenceBuilderUFix64NumberFieldTest.kt (2)
43-45: Approved: Improved readability of the assertion.The reformatting of the assertion improves readability while maintaining the test's functionality. This change aligns well with modern Kotlin style guidelines for multi-line statements.
For consistency with Kotlin's style guide, consider adding a trailing comma after the last parameter in the multi-line method call. This makes future additions easier and keeps the diff cleaner. Here's a suggested minor improvement:
Assertions - .assertThatThrownBy { UFix64NumberField("invalidValue").decodeToAny() } + .assertThatThrownBy { UFix64NumberField("invalidValue").decodeToAny() }, .isInstanceOf(NumberFormatException::class.java)
Line range hint
1-45: Well-structured and comprehensive test suite.This test file provides a robust set of test cases for the
UFix64NumberFieldclass, covering various scenarios including normal cases, edge cases, and error handling. The recent formatting improvement aligns well with the PR's objective of enhancing code readability and addressing lint issues throughout the codebase.Consider adding a test case for the maximum allowed value for UFix64 to ensure the field handles the upper bound correctly. This would further strengthen the test suite's coverage.
java-example/build.gradle.kts (1)
Line range hint
1-58: Overall assessment of build.gradle.kts updatesThe changes in this file align well with the PR objectives:
- Upgrading to the latest major release of Kotlin (2.0.21).
- Updating dependencies (JUnit Jupiter to 5.11.2).
- Adjusting build configurations to match Kotlin 2.0 requirements.
These updates should improve the project's compatibility with the latest Kotlin features and testing frameworks. However, given the significance of the Kotlin 2.0 upgrade, it's crucial to thoroughly test the entire project to ensure compatibility and stability.
Recommendations:
- Run a full build and test suite to verify everything works as expected with the new versions.
- Review the Kotlin 2.0 migration guide to identify and address any potential issues not caught by the compiler.
- Update any CI/CD pipelines to use Kotlin 2.0 and JDK 21.
- Consider updating other dependencies that might benefit from or require these upgrades.
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/executeScript/ExecuteScriptAccessAPIConnector.kt (3)
14-22: Improved readability with better formatting.The changes to the
executeSimpleScriptmethod have improved its readability. The new formatting breaks down the chained method calls into separate lines, making the code easier to understand and maintain.Consider adding a blank line before the
returnstatement to further improve readability:fun executeSimpleScript(): FlowScriptResponse { val loadedScript = ExamplesUtils.loadScriptContent("cadence/execute_simple_script_example.cdc") + return accessAPI .simpleFlowScript { script { loadedScript } arg { JsonCadenceBuilder().int(5) } }.let { response -> when (response) { is FlowAccessApi.AccessApiCallResponse.Success -> response.data is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable) } } }
29-37: Improved formatting consistency and potential hardcoded value concern.The changes to the
executeComplexScriptmethod have improved its readability and consistency with theexecuteSimpleScriptmethod. The new formatting breaks down the chained method calls into separate lines, making the code easier to understand and maintain.Consider the following improvements:
Add a blank line before the
returnstatement for consistency with the suggested change inexecuteSimpleScript.The hardcoded address
"0x84221fe0294044d7"might be better as a configurable parameter or constant. This would improve flexibility and make the code more maintainable. Consider refactoring it as follows:companion object { private const val DEFAULT_ADDRESS = "0x84221fe0294044d7" } fun executeComplexScript(address: String = DEFAULT_ADDRESS): FlowScriptResponse { val loadedScript = ExamplesUtils.loadScriptContent("cadence/execute_complex_script_example.cdc") return accessAPI .simpleFlowScript { script { loadedScript } arg { JsonCadenceBuilder().address(address) } }.let { response -> when (response) { is FlowAccessApi.AccessApiCallResponse.Success -> response.data is FlowAccessApi.AccessApiCallResponse.Error -> throw Exception(response.message, response.throwable) } } }This change allows the method to be more flexible while still providing a default value.
Line range hint
1-48: Overall structure improvements and unrelated data class.The formatting changes have been consistently applied to both
executeSimpleScriptandexecuteComplexScriptmethods, improving the overall readability and maintainability of the class.Consider the following suggestions for further improvements:
The
StorageInfodata class at the end of the file seems unrelated to the main functionality of theExecuteScriptAccessAPIConnectorclass. If it's not used within this file, consider moving it to a separate file or to a more appropriate location where it's actually used.If
StorageInfois indeed related to this class, consider adding a method that utilizes it to make the relationship clear. Otherwise, its presence here might be confusing for other developers.To improve the overall structure, you might want to add some documentation comments (KDoc) to the class and its methods, explaining their purpose and usage. This would enhance the maintainability and usability of the code.
Example:
/** * Executes Flow scripts using the Flow Access API. * * @property accessAPI The Flow Access API used for script execution. */ class ExecuteScriptAccessAPIConnector( private val accessAPI: FlowAccessApi ) { /** * Executes a simple script that takes an integer argument. * * @return The response from the executed script. * @throws Exception if the script execution fails. */ fun executeSimpleScript(): FlowScriptResponse { // ... (existing code) } /** * Executes a complex script that takes an address as an argument. * * @return The response from the executed script. * @throws Exception if the script execution fails. */ fun executeComplexScript(): FlowScriptResponse { // ... (existing code) } }These changes would further improve the code's structure and documentation.
.github/workflows/ci-examples-pull-request.yml (1)
Flow Emulator Service Removal Affects Existing Tests
The flow emulator service has been commented out in the CI workflow, but it is still referenced in multiple test files. This change may cause integration tests and other build processes that depend on the emulator to fail.
kotlin-example/src/test/kotlin/org/onflow/examples/kotlin/getNetworkParams/GetNetworkParametersAccessAPIConnectorTest.ktsdk/src/main/kotlin/org/onflow/flow/sdk/models.ktjava-example/src/test/java/org/onflow/examples/java/getNetworkParams/GetNetworkParametersAccessAPIConnectorTest.javasdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionIntegrationTest.ktcommon/src/testFixtures/kotlin/org/onflow/flow/common/test/FlowEmulatorProjectExtension.kt- (and others as listed in the script output)
🔗 Analysis chain
Line range hint
8-18: Clarify the removal of the flow emulator service.The flow emulator service configuration has been commented out entirely. This change could have significant implications for the CI pipeline, particularly for integration tests or other build processes that may depend on the emulator.
Please clarify the reasoning behind removing the flow emulator service from this workflow. If it's no longer needed, ensure that all tests and build steps that previously relied on it have been updated accordingly. If it's still required, consider keeping it active or providing an alternative solution.
To verify the impact, we can check for any references to the emulator in the build and test steps:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to the flow emulator in gradle files and test files rg --type gradle --type kotlin --type java "emulator|3569"Length of output: 3995
🧰 Tools
🪛 actionlint
25-25: the runner of "actions/setup-java@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.editorconfig (1)
Line range hint
1-47: Consider reviewing the overall ktlint configuration strategy.The
.editorconfigfile disables numerous ktlint rules. While this provides flexibility, it also significantly deviates from ktlint's default style guide. Consider the following suggestions:
- Review each disabled rule and determine if it's necessary to disable it project-wide.
- Consider creating a custom ktlint ruleset that better fits your project's needs instead of disabling many rules.
- Document the reasons for disabling each rule, either in this file or in a separate coding style guide document.
This approach will help maintain a balance between flexibility and consistency in your codebase style.
sdk/src/main/kotlin/org/onflow/flow/sdk/script-dsl.kt (1)
46-50: New chainId property added - LGTM with a minor suggestionThe addition of the
chainIdproperty enhances the configurability of theScriptBuilderclass. This change is well-implemented and consistent with the existing code style.However, we can simplify the property declaration using Kotlin's concise syntax for properties with backing fields:
var chainId: FlowChainId = _chainId set(value) { field = value }This achieves the same result with less code, making it more maintainable.
sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionTest.kt (1)
Line range hint
1-126: Consider further Kotlin idiomatic improvementsWhile the refactoring of
createSampleFlowTransactionis excellent, there are opportunities for further improvements in the test class:
Consider using more concise assertions where applicable. For example,
assertTrue(condition)could be replaced withcondition.assertTrue()using Kotlin's extension functions.Some test methods could benefit from using Kotlin's string templates for more readable assertions. For instance:
assertEquals(true, id.toString().isNotEmpty())could become:
assertTrue("Transaction ID should not be empty: $id") { id.toString().isNotEmpty() }Consider using Kotlin's
withorapplyscope functions to group related assertions, improving readability.These suggestions aim to make the tests more idiomatic and easier to read, following the direction set by the
createSampleFlowTransactionrefactoring.sdk/src/test/kotlin/org/onflow/flow/sdk/ScriptTest.kt (1)
32-33: Excellent refactoring to idiomatic Kotlin syntax!The change from a block body to an expression body for the
marshallmethod is a great improvement. It makes the code more concise and aligns with Kotlin's idiomatic style for simple method bodies. This refactoring maintains the existing functionality while enhancing readability.Consider applying similar refactoring to other simple methods in the codebase for consistency. This aligns well with the PR's objective of updating the codebase to newer Kotlin standards.
sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (2)
Line range hint
194-209: LGTM: Consistent refactoring to idiomatic KotlinThe
getAccountByAddressmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.Consider: As this method is deprecated, consider removing it in a future update to reduce maintenance overhead and potential confusion for users of the API.
Line range hint
246-261: LGTM: Consistent refactoring to idiomatic Kotlin, but consider improving error handlingThe
executeScriptAtLatestBlockmethod has been refactored to use expression body syntax, consistent with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.Consider: Review the error handling approach. Printing error messages and stack traces to the console might not be suitable for production code. Consider using a logging framework or a more robust error reporting mechanism.
sdk/src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (7)
50-56: LGTM: Improved method call readabilityThe restructuring of the
api.getLatestBlockHeader()call enhances readability. Each parameter of thenewBuilder()call is now on a separate line, making the structure clearer and easier to modify in the future.Consider applying this formatting consistently to all similar method calls throughout the file for improved overall readability and maintainability.
385-386: Consider completing the restructuring for consistencyWhile the initial restructuring of the
api.getAccountAtBlockHeight()call improves readability, it's not fully consistent with the pattern established in previous changes. For full consistency, consider moving each parameter of thenewBuilder()call to a separate line.Here's a suggested modification:
api.getAccountAtBlockHeight( Access.GetAccountAtBlockHeightRequest .newBuilder() - .setAddress(addresss.byteStringValue) - .setBlockHeight(height) - .build() + .setAddress(addresss.byteStringValue) + .setBlockHeight(height) + .build() )
415-416: Consider completing the restructuring for consistencyThe initial restructuring of the
api.executeScriptAtLatestBlock()call improves readability, but it's not fully consistent with the pattern established in previous changes. For full consistency, consider moving each parameter of thenewBuilder()call to a separate line.Here's a suggested modification:
api.executeScriptAtLatestBlock( Access.ExecuteScriptAtLatestBlockRequest .newBuilder() - .setScript(script.byteStringValue) - .addAllArguments(arguments) - .build() + .setScript(script.byteStringValue) + .addAllArguments(arguments) + .build() )
441-442: Consider completing the restructuring for consistencyThe initial restructuring of the
api.executeScriptAtBlockID()call improves readability, but it's not fully consistent with the pattern established in previous changes. For full consistency, consider moving each parameter of thenewBuilder()call to a separate line.Here's a suggested modification:
api.executeScriptAtBlockID( Access.ExecuteScriptAtBlockIDRequest .newBuilder() - .setBlockId(blockId.byteStringValue) - .setScript(script.byteStringValue) - .addAllArguments(arguments) - .build() + .setBlockId(blockId.byteStringValue) + .setScript(script.byteStringValue) + .addAllArguments(arguments) + .build() )
468-469: Consider completing the restructuring for consistencyThe initial restructuring of the
api.executeScriptAtBlockHeight()call improves readability, but it's not fully consistent with the pattern established in previous changes. For full consistency, consider moving each parameter of thenewBuilder()call to a separate line.Here's a suggested modification:
api.executeScriptAtBlockHeight( Access.ExecuteScriptAtBlockHeightRequest .newBuilder() - .setBlockHeight(height) - .setScript(script.byteStringValue) - .addAllArguments(arguments) - .build() + .setBlockHeight(height) + .setScript(script.byteStringValue) + .addAllArguments(arguments) + .build() )
495-496: Consider completing the restructuring for consistencyThe initial restructuring of the
api.getEventsForHeightRange()call improves readability, but it's not fully consistent with the pattern established in previous changes. For full consistency, consider moving each parameter of thenewBuilder()call to a separate line.Here's a suggested modification:
api.getEventsForHeightRange( Access.GetEventsForHeightRangeRequest .newBuilder() - .setType(type) - .setStartHeight(range.start) - .setEndHeight(range.endInclusive) - .build() + .setType(type) + .setStartHeight(range.start) + .setEndHeight(range.endInclusive) + .build() )
522-523: Consider completing the restructuring for consistencyThe initial restructuring of the
api.getEventsForBlockIDs()call improves readability, but it's not fully consistent with the pattern established in previous changes. For full consistency, consider moving each parameter of thenewBuilder()call to a separate line.Here's a suggested modification:
api.getEventsForBlockIDs( Access.GetEventsForBlockIDsRequest .newBuilder() - .setType(type) - .addAllBlockIds(ids.map { it.byteStringValue }) - .build() + .setType(type) + .addAllBlockIds(ids.map { it.byteStringValue }) + .build() )build.gradle.kts (2)
65-65: Maintain ConsistentJvmTargetImportsIn line 65, you use
JvmTarget.JVM_21directly, whereas in line 77, you reference it via the full packageorg.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_21. For consistency and readability, consider using the same import statement in both places.Apply this diff to standardize the import usage:
- jvmTarget.set(org.jetbrains.kotlin.gradle.dsl.JvmTarget.JVM_21) // Set JVM target to 21 + jvmTarget.set(JvmTarget.JVM_21) // Set JVM target to 21Ensure that you have the following import at the top of your file:
import org.jetbrains.kotlin.gradle.dsl.JvmTargetAlso applies to: 77-77
72-72: Remove or Update Commented-Out Dokka Plugin DependencyThere's a commented-out Dokka plugin dependency:
//dokkaHtmlPlugin"("org.jetbrains.dokka:kotlin-as-java-plugin:2.0.0-Beta")If this dependency is no longer needed, consider removing it to keep the build script clean. If it's required, uncomment and ensure it references the correct version compatible with Dokka
2.0.0-Beta.Apply this diff if the dependency is unnecessary:
- //dokkaHtmlPlugin"("org.jetbrains.dokka:kotlin-as-java-plugin:2.0.0-Beta")sdk/build.gradle.kts (1)
34-34: Consider stability implications of using Dokka 2.0.0-BetaThe Dokka HTML plugin has been updated to
2.0.0-Beta. Since this is a beta version, it may contain bugs or unstable features. Assess whether using a beta version aligns with the project's stability requirements.If stability is a priority, consider using the latest stable release of Dokka.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (55)
- .editorconfig (1 hunks)
- .github/workflows/ci-examples-pull-request.yml (1 hunks)
- .github/workflows/ci-sdk-pull-request.yml (1 hunks)
- build.gradle.kts (4 hunks)
- common/build.gradle.kts (2 hunks)
- common/src/testFixtures/kotlin/org/onflow/flow/common/test/FlowTestUtil.kt (4 hunks)
- gradle.properties (1 hunks)
- gradle/wrapper/gradle-wrapper.properties (1 hunks)
- java-example/build.gradle.kts (3 hunks)
- kotlin-example/build.gradle.kts (3 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/ExamplesUtils.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/deployContract/DeployContractExample.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/executeScript/ExecuteScriptAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getAccount/GetAccountAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getBlock/GetBlockAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getCollection/GetCollectionAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getEvent/GetEventAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getExecutionData/GetExecutionDataAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getNetworkParams/GetNetworkParametersAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getTransaction/GetTransactionAccessAPIConnector.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/streaming/streamEventsReconnect/SubscribeEventsReconnectExample.kt (4 hunks)
- sdk/build.gradle.kts (2 hunks)
- sdk/src/intTest/org/onflow/flow/sdk/ExposeAccountKeyIssueTest.kt (3 hunks)
- sdk/src/intTest/org/onflow/flow/sdk/IntegrationTestUtils.kt (1 hunks)
- sdk/src/intTest/org/onflow/flow/sdk/cadence/JsonCadenceTest.kt (2 hunks)
- sdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionCreationTest.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/cadence/json-cadence-marshalling.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/cadence/json-cadence.kt (14 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/errors.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/extensions.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (24 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (19 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/script-dsl.kt (2 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/transaction-dsl.kt (7 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/ScriptTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/JsonCadenceBuilderFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/JsonCadenceBuilderOptionalFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/number/JsonCadenceBuilderUFix64NumberFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/number/JsonCadenceBuilderUInt64NumberFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/number/JsonCadenceBuilderUInt8NumberFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImplTest.kt (26 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (26 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowAccountKeyTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowAccountTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowBlockTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowCollectionGuaranteeTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowCollectionTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowEventTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowIdTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionProposalKeyTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionResultTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionSignatureTest.kt (2 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionTest.kt (1 hunks)
✅ Files skipped from review due to trivial changes (20)
- gradle.properties
- gradle/wrapper/gradle-wrapper.properties
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/deployContract/DeployContractExample.kt
- sdk/src/intTest/org/onflow/flow/sdk/ExposeAccountKeyIssueTest.kt
- sdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionCreationTest.kt
- sdk/src/main/kotlin/org/onflow/flow/sdk/FlowAccessApi.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/FlowAccessApiTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/JsonCadenceBuilderFieldTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/JsonCadenceBuilderOptionalFieldTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/number/JsonCadenceBuilderUInt64NumberFieldTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImplTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowAccountTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowBlockTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowCollectionGuaranteeTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowCollectionTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowEventTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowIdTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionProposalKeyTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionResultTest.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionSignatureTest.kt
🧰 Additional context used
🔇 Additional comments (118)
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getExecutionData/GetExecutionDataAccessAPIConnector.kt (1)
5-7: LGTM: Class declaration formatting improvedThe multi-line formatting of the class declaration enhances readability. The use of the
internalvisibility modifier and theprivateconstructor parameter are appropriate for this utility class.kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getNetworkParams/GetNetworkParametersAccessAPIConnector.kt (1)
9-12: Excellent refactoring to a more idiomatic Kotlin style!The change to a single-expression function syntax for
getNetworkParameters()is a great improvement. This refactoring:
- Enhances code readability by making the function more concise.
- Aligns with Kotlin's idiomatic style for simple functions.
- Maintains the existing logic and functionality.
The use of
whenexpression as a direct return value is particularly elegant in this context.kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getCollection/GetCollectionAccessAPIConnector.kt (2)
6-8: LGTM: Improved constructor formattingThe multi-line formatting of the constructor enhances readability and follows Kotlin best practices. This change makes the code more maintainable, especially if additional parameters are added in the future.
6-12: Overall improvements enhance code qualityThe changes made to this file have successfully improved code readability and adherence to Kotlin best practices. The multi-line constructor formatting and the conversion of
getCollectionByIdto a single-expression function make the code more concise and maintainable. These improvements align well with the PR objectives of updating and enhancing the codebase.No functional changes were introduced, ensuring that existing behavior is preserved while improving code quality. Great job on these refactoring efforts!
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/ExamplesUtils.kt (1)
13-14: LGTM! Improved function conciseness.The refactoring of
loadScriptContentto use expression body syntax enhances code readability without changing its functionality. This change aligns well with modern Kotlin coding practices for concise function definitions.kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getTransaction/GetTransactionAccessAPIConnector.kt (3)
8-12: Excellent refactoring to expression body!The change to use an expression body for the
getTransactionmethod improves code conciseness and readability while maintaining the same functionality. This refactoring aligns well with Kotlin best practices for simple functions.
14-18: Consistent refactoring applied togetTransactionResult!The
getTransactionResultmethod has been refactored in the same manner asgetTransaction, using an expression body. This change maintains consistency across the class and improves overall code readability. The functionality remains intact while adopting a more idiomatic Kotlin style.
Line range hint
1-20: Overall excellent refactoring ofGetTransactionAccessAPIConnectorThe changes made to this file are consistent and improve the code quality:
- Both
getTransactionandgetTransactionResultmethods have been refactored to use expression bodies.- The refactoring makes the code more concise and idiomatic Kotlin without changing the functionality.
- Error handling and success cases remain intact for both methods.
These changes align well with the PR objectives of updating and improving the codebase. Great job on maintaining consistency and enhancing readability!
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getAccount/GetAccountAccessAPIConnector.kt (2)
9-13: LGTM! Improved code readability with expression body syntax.The refactoring of
getAccountAtLatestBlockto use expression body syntax is a positive change. It makes the code more concise and idiomatic in Kotlin while maintaining the same functionality and error handling. This change aligns well with Kotlin best practices.
15-19: LGTM! Consistent refactoring improves overall code quality.The refactoring of
getAccountAtBlockHeightto use expression body syntax is consistent with the changes made togetAccountAtLatestBlock. This systematic approach to code improvement enhances readability across the class while maintaining functionality and error handling. Well done on keeping the refactoring consistent!kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getBlock/GetBlockAccessAPIConnector.kt (2)
16-20: LGTM! Improved method conciseness.The refactoring of
getBlockByIDto use expression body syntax is a good improvement. It makes the code more concise and aligns with modern Kotlin best practices, enhancing readability without altering the method's behavior.
22-26: LGTM! Consistent improvement in method conciseness.The refactoring of
getBlockByHeightto use expression body syntax is consistent with the changes made togetBlockByID. This improvement maintains a uniform coding style across the class, enhancing overall readability and adhering to modern Kotlin best practices.sdk/src/main/kotlin/org/onflow/flow/sdk/extensions.kt (1)
23-27: LGTM! Improved code readability.The changes to the
LocalDateTime.asTimestamp()function are purely formatting-related and improve the code's readability. The multi-line format for theTimestamp.newBuilder()call makes the code structure clearer without altering its functionality. This adheres to good coding practices and makes the code easier to maintain.kotlin-example/build.gradle.kts (2)
Line range hint
54-56: Review duplicates strategy for Copy tasks.The addition of a
duplicatesStrategyfor Copy tasks is noteworthy:tasks.withType<Copy> { duplicatesStrategy = DuplicatesStrategy.EXCLUDE }This setting prevents overwriting existing files during copy operations, which can be beneficial in preventing unintended file replacements. However, it may also lead to missing updates if newer versions of files are not copied due to existing files with the same name.
To ensure this change doesn't negatively impact your build process:
- Verify that all necessary files are still being copied correctly after this change.
- Check if there are any specific Copy tasks in your build that might require a different strategy.
Run a build and inspect the output files:
#!/bin/bash # Description: Run a build and list output directories to verify copy operations ./gradlew build echo "Listing build output directories:" find build -type dIf you notice any missing or outdated files in the output, you may need to adjust the strategy for specific tasks or reconsider this global setting.
38-39: Approve JUnit version update.The update of JUnit dependencies from version 5.7.0 to 5.11.2 is a good improvement. This update likely brings bug fixes and potentially new features that can enhance your testing capabilities.
To ensure that this update doesn't introduce any unexpected issues, please verify that all tests still pass with the new JUnit version. Run the following command to check:
If any tests fail, please investigate and address the issues before merging this PR.
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/getEvent/GetEventAccessAPIConnector.kt (5)
8-12: LGTM! Improved code conciseness.The refactoring to use an expression body enhances readability while maintaining the original functionality. This change aligns well with Kotlin's idiomatic style for concise function definitions.
14-18: LGTM! Consistent refactoring applied.The changes to
getEventsForBlockIdsmirror those in the previous method, maintaining a consistent style throughout the class. The refactoring improves readability without altering the method's behavior.
20-24: LGTM! Consistent refactoring pattern maintained.The changes to
getTransactionResultfollow the same refactoring pattern as the previous methods. This consistency in applying the expression body syntax across all methods enhances the overall readability of the class.
26-27: LGTM! Simplified wrapper method.The refactoring of
getAccountCreatedEventsto use an expression body is particularly effective for this simple wrapper method. It clearly shows the method's purpose of providing a specialized call togetEventsForHeightRange.
Line range hint
1-28: Overall improvement in code style and readability.The refactoring applied consistently across all methods in
GetEventAccessAPIConnectorhas resulted in more concise and readable code. By using expression bodies, the code now better aligns with Kotlin's idiomatic style without introducing any functional changes. This update enhances maintainability while preserving the original behavior of the class.java-example/build.gradle.kts (4)
46-46: JUnit Jupiter Engine updated to 5.11.2This update is consistent with the JUnit Jupiter API update. Keeping the API and Engine versions in sync is crucial for proper test execution.
This change is linked to the JUnit Jupiter API update. The verification steps mentioned in the previous comment apply to this change as well.
45-45: JUnit Jupiter API updated to 5.11.2The update to JUnit Jupiter API 5.11.2 aligns with the PR objective of updating dependencies. This minor version update should introduce new features and bug fixes without breaking changes.
Please ensure that:
- All existing tests run successfully with the new JUnit version.
- There are no deprecation warnings related to JUnit usage.
Run the following script to verify the JUnit update:
#!/bin/bash # Description: Verify JUnit update and test execution # Test: Run all tests and check for any failures ./gradlew :java-example:test # Test: Check for any JUnit-related deprecation warnings ./gradlew :java-example:test --info | grep -i "junit" | grep -i "deprecat"
23-25: KotlinCompile task configuration updated for Kotlin 2.0The changes to the KotlinCompile task configuration align with the new Kotlin 2.0 Gradle plugin API. The use of
compilerOptionsandJvmTarget.JVM_21provides better type safety.Please ensure that:
- The project builds successfully with these new compiler options.
- The generated bytecode is compatible with JVM 21 as intended.
Run the following script to verify the Kotlin compiler options:
#!/bin/bash # Description: Verify Kotlin compiler options and bytecode version # Test: Check Kotlin compiler options ./gradlew :java-example:compileKotlin --dry-run # Test: Verify bytecode version of compiled Kotlin classes ./gradlew :java-example:compileKotlin find java-example/build/classes/kotlin/main -name "*.class" | xargs javap -verbose | grep "major version"
7-7: Kotlin plugin version updated to 2.0.21The update to Kotlin 2.0.21 aligns with the PR objective of upgrading to the latest major release. This is a significant update that may introduce breaking changes.
Please ensure that:
- All code in the project is compatible with Kotlin 2.0.
- All dependencies are compatible with Kotlin 2.0.
- The project builds and runs successfully with this new version.
Run the following script to check for potential compatibility issues:
.github/workflows/ci-sdk-pull-request.yml (1)
Line range hint
8-18: Clarify the removal of flow-emulator serviceThe flow-emulator service configuration has been commented out. This change could potentially impact integration tests or other parts of the build process that rely on the Flow emulator.
Could you please clarify:
- Why was the flow-emulator service removed?
- Are there any parts of the build or test process that previously depended on this service?
- Have alternative arrangements been made if the emulator is still needed?
To check for potential impacts, run:
#!/bin/bash # Search for references to the Flow emulator in the codebase rg -i 'flow.*emulator|emulator.*port|localhost:3569' --type kotlin --type java --type gradle🧰 Tools
🪛 actionlint
25-25: the runner of "actions/setup-java@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/ci-examples-pull-request.yml (2)
27-30: Approve Java setup changes with a compatibility check recommendation.The updates to the Java setup step align with the PR objective of updating dependencies. The changes include:
- Updating Java version to '21.0.1'
- Changing distribution to 'temurin'
- Specifying 'x64' architecture
These modifications are appropriate for keeping the CI environment up-to-date.
To ensure these changes don't introduce any compatibility issues, please verify that all examples and tests run successfully with this new Java setup. You can confirm this by checking the subsequent steps in this workflow for any failures.
Line range hint
1-76: Recommend thorough testing of the entire workflow.While no other significant changes are visible in the workflow file, the updates to the Java setup and the removal of the flow emulator service could potentially affect other parts of the CI process.
To ensure the integrity of the CI pipeline, please perform a comprehensive test of the entire workflow:
- Verify that all build steps complete successfully.
- Ensure that all unit and integration tests pass.
- Check that the test result publishing steps work as expected.
You can monitor the workflow runs in the GitHub Actions tab to confirm that all steps are executing correctly with the new configuration.
🧰 Tools
🪛 actionlint
25-25: the runner of "actions/setup-java@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.editorconfig (2)
7-7: Updatebuild.gradle.ktsas mentioned in the comment.The comment "be sure to update build.gradle.kts in the root as well" is crucial. Ensure that the
build.gradle.ktsfile is updated to reflect these ktlint configuration changes.Let's verify if the
build.gradle.ktsfile has been updated:#!/bin/bash # Description: Check if build.gradle.kts has been updated with ktlint configurations # Search for ktlint configuration in build.gradle.kts echo "Ktlint configuration in build.gradle.kts:" rg --type kotlin 'ktlint\s*\{' -A 10 build.gradle.kts # Check if the new rule is mentioned in build.gradle.kts echo -e "\nChecking for chain-method-continuation rule:" rg --type kotlin 'chain-method-continuation' build.gradle.ktsThis script will help us confirm if the necessary updates have been made to
build.gradle.kts. If the changes are not present, please update the file to maintain consistency with the.editorconfigsettings.
10-10: Consider the implications of disabling the chain method continuation rule.The addition of
ktlint_standard_chain_method_continuation = disabledaligns with the PR objective of updating the ktlint configuration. However, disabling this rule may have both positive and negative impacts:Pros:
- Allows more flexibility in code formatting for chained method calls.
- Consistent with the approach of disabling other ktlint rules in this file.
Cons:
- May lead to inconsistent formatting of chained method calls across the codebase.
- Could potentially reduce code readability if developers use vastly different styles for method chaining.
To ensure this change doesn't conflict with any existing style guide or cause inconsistencies, let's verify the current usage of chained method calls in the codebase:
This script will help us understand the current practices in the codebase regarding method chaining. If there's a consistent style already in use, we might want to consider enforcing it rather than disabling the rule entirely.
✅ Verification successful
Disabling the chain method continuation rule aligns with the project's flexible approach to code style.
The addition of
ktlint_standard_chain_method_continuation = disabledin the .editorconfig file is consistent with the project's overall approach to ktlint rules. Many other ktlint rules are also disabled, indicating a preference for a more flexible code style that may deviate from ktlint's default recommendations.While method chaining is used throughout the codebase, particularly in builder patterns and functional-style operations, disabling this rule allows developers more freedom in formatting these chains according to their preferences or the specific needs of the code structure.
This change:
- Provides consistency with the project's existing ktlint configuration
- Allows for more flexible formatting of method chains
- May lead to improved readability in complex chaining scenarios, where strict adherence to the rule might result in less intuitive formatting
However, it's important to note that without the rule, there's a potential for inconsistent formatting of method chains across the codebase. To mitigate this, consider:
- Documenting the preferred method chaining style in the project's coding guidelines
- Encouraging code reviews to maintain consistency in method chain formatting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing chained method call patterns in Kotlin files # Search for method chaining patterns in Kotlin files echo "Chained method call patterns in the codebase:" rg --type kotlin '(\.\s*\w+\s*\([^)]*\)\s*){2,}' -n # Count occurrences of different chaining styles echo -e "\nCounts of different chaining styles:" rg --type kotlin '(\.\s*\w+\s*\([^)]*\)\s*){2,}' | sed -E 's/.*(\.)(.*)$/\1\2/' | sort | uniq -c | sort -nrLength of output: 343
Script:
#!/bin/bash # Description: Use ast-grep to find chained method calls in Kotlin files # Find all chained method calls in Kotlin files ast-grep --lang kotlin --pattern 'call_expression: (call_expression . call_expression)' # Alternatively, search for lines with multiple consecutive dot operators rg --type kotlin '(\.\w+){2,}' -nLength of output: 267855
Script:
#!/bin/bash # Search for method chaining patterns and ktlint/code style references echo "Method chaining patterns:" rg --type kotlin '(\.\s*\w+\s*\([^)]*\)\s*){2,}' -n echo -e "\nReferences to ktlint or code style:" rg --type kotlin "ktlint|code style" -n echo -e "\nChecking .editorconfig for ktlint rules:" cat .editorconfig | grep ktlintLength of output: 19568
sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowAccountKeyTest.kt (1)
Line range hint
15-22: Excellent refactoring to improve readability!The use of the builder pattern for initializing
protobufKeyis a great improvement. This change enhances code readability and maintainability without altering the functionality. It's now easier to see each property being set, which will make future modifications or additions more straightforward. This style is also more consistent with Kotlin's idiomatic use of the builder pattern.sdk/src/main/kotlin/org/onflow/flow/sdk/script-dsl.kt (1)
13-14: Improved function syntax - LGTM!The
executeScriptfunction has been refactored to use a single-expression syntax, which is more idiomatic in Kotlin. This change simplifies the code without altering its functionality, making it more concise and easier to read.sdk/src/intTest/org/onflow/flow/sdk/IntegrationTestUtils.kt (1)
24-27: Approved: Improved code conciseness with expression bodyThe refactoring of the
handleResultfunction to use an expression body is a good improvement. This change:
- Enhances code readability by making the function more concise.
- Utilizes Kotlin's expressive features effectively.
- Maintains the original functionality without introducing any logical changes.
Great job on this clean and effective refactoring!
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/streaming/streamEventsReconnect/SubscribeEventsReconnectExample.kt (5)
6-6: LGTM: Import statement added foronTimeoutThe addition of the import statement for
kotlinx.coroutines.selects.onTimeoutis appropriate and consistent with its usage in theprocessEventsWithReconnectmethod.
23-27: LGTM: RefactoredgetLatestBlockHeaderto a single-expression functionThe refactoring of
getLatestBlockHeaderto a single-expression function improves code readability and conciseness. The use of thewhenexpression for handling the API response is appropriate and maintains the original functionality.
68-71: LGTM: Improved clarity inonTimeoutcallThe explicit naming of the
timeMillisparameter in theonTimeoutcall enhances code readability. This change makes the purpose of the 1000L value more evident without altering the functionality.
101-102: LGTM: Refactoredreconnectto a single-expression functionThe refactoring of the
reconnectmethod to a single-expression function enhances code readability and conciseness. The functionality remains unchanged, and the method signature (parameters and return type) is preserved.
Line range hint
1-103: Overall: Excellent refactoring and code improvementsThe changes made to this file consistently enhance code readability and maintainability without altering the core functionality. The refactoring to single-expression functions and the improved clarity in method calls align well with modern Kotlin coding practices. These modifications make the code more concise and easier to understand, which is particularly beneficial for a complex class handling event streaming and reconnection logic.
sdk/src/test/kotlin/org/onflow/flow/sdk/models/FlowTransactionTest.kt (2)
110-111: Excellent refactoring to idiomatic Kotlin!The
createSampleFlowTransactionfunction has been elegantly refactored to a single-expression function. This change:
- Improves code conciseness and readability.
- Aligns with modern Kotlin best practices.
- Maintains the existing functionality, ensuring test integrity.
Great job on this clean and effective improvement!
Line range hint
1-126: Summary: Successful refactoring with room for further improvementsThe changes in this file, particularly the refactoring of
createSampleFlowTransaction, represent a positive step towards more idiomatic Kotlin code. The refactoring improves readability without altering functionality, which is crucial for maintaining test integrity.While the current changes are approved, there's potential for further enhancements to the test class to align it more closely with Kotlin best practices. These additional improvements could make the tests even more readable and maintainable.
Overall, this is a solid contribution that aligns well with the PR's objective of updating and improving the codebase.
sdk/src/main/kotlin/org/onflow/flow/sdk/errors.kt (2)
10-12: LGTM: Improved enum class constructor formattingThe change in the
FlowErrorenum class constructor formatting enhances readability without affecting functionality. This adjustment aligns with Kotlin style guidelines for multiline declarations.
Line range hint
1-158: Summary: Good progress on code cleanup, consider documentation updatesThe changes in this file align well with the PR objectives of updating dependencies and addressing lint issues. The improved formatting enhances readability, and the deprecation of unused constants helps in cleaning up the codebase.
To further improve this update:
- Consider updating the project's documentation to reflect these changes, especially the deprecations.
- Ensure that the changelog or release notes mention these deprecations and any potential impact on users.
- If possible, provide a migration guide for users who might be using the deprecated constants in their code.
Overall, these changes contribute positively to the maintainability of the SDK.
sdk/src/intTest/org/onflow/flow/sdk/cadence/JsonCadenceTest.kt (2)
50-51: LGTM: Improved function concisenessThe refactoring of
loadScriptContentto a single-expression function is a good improvement. It makes the code more concise and idiomatic Kotlin without changing its functionality.
134-137: LGTM: Enhanced readability of complex expressionThe reformatting of the success case in the
whenexpression significantly improves readability. Breaking down the chained method calls into separate lines makes the data flow easier to follow without altering the functionality.common/src/testFixtures/kotlin/org/onflow/flow/common/test/FlowTestUtil.kt (5)
31-32: Improved readability indeployContractsfunctionThe changes to the
contractArgsandcontractAddArgsvariables have improved the code's readability. The sorting and joining operations are now more concise and easier to understand.Also applies to: 34-34, 37-38, 40-40
87-103: Enhanced code structure increateAccountfunctionThe reformatting of the
transactionResultassignment has significantly improved the code structure. The vertical alignment of method calls makes it easier to read and understand the transaction setup process.
118-118: Minor formatting improvement in error handlingThe addition of a line break in the error response construction improves the readability of the code without changing its functionality.
207-207: Improved readability inrunFlowfunctionThe reformatting of the list of potential executable paths has enhanced the code's readability. The vertical alignment makes it easier to scan and understand the different path options.
Line range hint
1-258: Overall improvements align with PR objectivesThe changes in this file consistently improve code readability and formatting without altering functionality. These improvements align well with the PR objective of addressing outstanding lint issues throughout the codebase. The enhanced readability will contribute to better code maintainability in the long run.
sdk/src/main/kotlin/org/onflow/flow/sdk/cadence/json-cadence-marshalling.kt (4)
57-60: Improved code conciseness using expression bodiesThe changes to
unmarshallandmarshallmethods in theJsonCadenceConverterinterface enhance code readability by using Kotlin's expression body syntax. This modification simplifies the method definitions without altering their functionality.
271-274: Consistent use of expression body inenummethodThe
enummethod in theJsonCadenceParserclass has been updated to use an expression body, consistent with the changes made to theJsonCadenceConverterinterface. This change improves code conciseness while maintaining the existing functionality.
Line range hint
65-104: Enhanced Java interoperability with@JvmStaticannotationsThe addition of
@JvmStaticannotations to several methods in theJsonCadenceMarshallingobject improves Java interoperability. These annotations ensure that the annotated methods are accessible as static methods from Java code, enhancing the usability of the SDK for Java developers without affecting Kotlin usage.
Line range hint
1-314: Summary: Improved code style and Java interoperabilityThe changes in this file consistently enhance code conciseness by utilizing Kotlin's expression body syntax and improve Java interoperability through the addition of
@JvmStaticannotations. These modifications align with modern Kotlin practices and enhance the SDK's usability for both Kotlin and Java developers. The functionality remains unchanged, and the code quality has been improved.sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (22)
18-19: LGTM: Improved readability of class declarationThe split of the class declaration into two lines enhances readability and follows Kotlin style guidelines.
27-36: LGTM: Refactored to more idiomatic KotlinThe
pingmethod has been refactored to use expression body syntax, making it more concise and idiomatic Kotlin while maintaining the same functionality.
39-50: LGTM: Consistent refactoring to idiomatic KotlinThe
getLatestBlockHeadermethod has been refactored to use expression body syntax, consistent with the previous changes. This improves code consistency and readability while maintaining the same functionality.
Line range hint
52-67: LGTM: Consistent refactoring to idiomatic KotlinThe
getBlockHeaderByIdmethod has been refactored to use expression body syntax, maintaining consistency with previous changes. This enhances code readability while preserving the original functionality.
Line range hint
69-84: LGTM: Consistent refactoring to idiomatic KotlinThe
getBlockHeaderByHeightmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
86-97: LGTM: Consistent refactoring to idiomatic KotlinThe
getLatestBlockmethod has been refactored to use expression body syntax, maintaining consistency with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
Line range hint
99-114: LGTM: Consistent refactoring to idiomatic KotlinThe
getBlockByIdmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
Line range hint
116-131: LGTM: Consistent refactoring to idiomatic KotlinThe
getBlockByHeightmethod has been refactored to use expression body syntax, consistent with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
Line range hint
133-148: LGTM: Consistent refactoring to idiomatic KotlinThe
getCollectionByIdmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
150-161: LGTM: Consistent refactoring to idiomatic KotlinThe
sendTransactionmethod has been refactored to use expression body syntax, maintaining consistency with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
Line range hint
163-178: LGTM: Consistent refactoring to idiomatic KotlinThe
getTransactionByIdmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
180-191: LGTM: Consistent refactoring to idiomatic KotlinThe
getTransactionResultByIdmethod has been refactored to use expression body syntax, consistent with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
Line range hint
211-226: LGTM: Consistent refactoring to idiomatic KotlinThe
getAccountAtLatestBlockmethod has been refactored to use expression body syntax, maintaining consistency with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
Line range hint
228-244: LGTM: Consistent refactoring to idiomatic KotlinThe
getAccountByBlockHeightmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
Line range hint
263-276: LGTM: Consistent refactoring to idiomatic KotlinThe
executeScriptAtBlockIdmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
Line range hint
278-291: LGTM: Consistent refactoring to idiomatic KotlinThe
executeScriptAtBlockHeightmethod has been refactored to use expression body syntax, maintaining consistency with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
Line range hint
293-306: LGTM: Consistent refactoring to idiomatic KotlinThe
getEventsForHeightRangemethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
Line range hint
308-320: LGTM: Consistent refactoring to idiomatic KotlinThe
getEventsForBlockIdsmethod has been refactored to use expression body syntax, consistent with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
322-332: LGTM: Consistent refactoring to idiomatic KotlinThe
getNetworkParametersmethod has been refactored to use expression body syntax, aligning with the previous changes. This refactoring improves code consistency and readability while maintaining the original functionality.
334-344: LGTM: Consistent refactoring to idiomatic KotlinThe
getLatestProtocolStateSnapshotmethod has been refactored to use expression body syntax, maintaining consistency with previous changes. This refactoring enhances code readability and follows Kotlin best practices while preserving the original functionality.
346-357: LGTM: Consistent refactoring to idiomatic KotlinThe
getTransactionsByBlockId,getTransactionResultsByBlockId, andgetExecutionResultByBlockIdmethods have been refactored to use expression body syntax, aligning with the previous changes. These refactorings improve code consistency and readability while maintaining the original functionality.Also applies to: 359-370, 372-387
Line range hint
1-506: Overall: Excellent refactoring to idiomatic KotlinThis PR has successfully refactored the entire
FlowAccessApiImplclass to use more idiomatic Kotlin syntax. The consistent use of expression body syntax across all methods has significantly improved code readability and maintainability. These changes align well with Kotlin best practices and make the codebase more concise without altering any functionality.Suggestions for future improvements:
- Consider removing the deprecated
getAccountByAddressmethod in a future update.- Review the error handling approach in the
executeScriptAtLatestBlockmethod, potentially replacing console printing with a more robust logging or error reporting mechanism.Great job on this systematic refactoring effort!
sdk/src/main/kotlin/org/onflow/flow/sdk/cadence/json-cadence.kt (11)
Line range hint
150-169: LGTM: Improved code style and readabilityThe changes to the
Fieldclass anddecodeToAny()function improve code readability and align with modern Kotlin style guidelines. The use of an expression body fordecodeToAny()simplifies the code without altering its functionality.
326-327: LGTM: Simplified function using expression bodyThe
CompositeValue.toMap()function has been refactored to use an expression body, which aligns with Kotlin's idiomatic practices and improves code conciseness without changing its functionality.
343-344: LGTM: Improved code style with expression bodyThe
toMap()function has been refactored to use an expression body, which enhances code readability and aligns with Kotlin's best practices while maintaining the same functionality.
356-371: LGTM: Consistent formatting improvementsThe class declarations for
OptionalField,BooleanField,StringField, andNumberFieldhave been reformatted to use a more modern Kotlin style. These changes enhance code readability and maintain consistency throughout the codebase without affecting functionality.
386-445: LGTM: Uniform formatting for NumberField subclassesAll NumberField subclasses (e.g.,
IntNumberField,UIntNumberField,Int8NumberField, etc.) have been reformatted to use a consistent, modern Kotlin style. This improves code readability and maintains a uniform structure across the codebase without altering any functionality.
447-453: LGTM: Improved ArrayField class styleThe
ArrayFieldclass has been updated with improved formatting and thehashCode()function now uses an expression body. These changes enhance code readability and adhere to Kotlin's best practices while maintaining the same functionality.
463-473: LGTM: Enhanced DictionaryField class styleThe
DictionaryFieldclass and its companion object have been updated with improved formatting. ThefromPairsandfromMapfunctions now use expression bodies, which aligns with Kotlin's idiomatic practices. These changes improve code readability while maintaining the same functionality.
Line range hint
477-519: LGTM: Consistent formatting improvementsThe class declarations for
DictionaryFieldEntry,AddressField,PathValue, andPathFieldhave been reformatted to use a more modern Kotlin style. ThehashCode()functions inPathValueandPathFieldnow use expression bodies. These changes enhance code readability and maintain consistency throughout the codebase without affecting functionality.
533-555: LGTM: Improved class declaration formattingThe class declarations for
CapabilityValue,CapabilityField,CompositeField, andCompositeAttributehave been reformatted to use a more modern Kotlin style. These changes enhance code readability and maintain consistency throughout the codebase without altering any functionality.
Line range hint
573-588: LGTM: Enhanced CompositeValue class formattingThe
CompositeValueclass declaration has been reformatted to use a more modern Kotlin style. This change improves code readability and maintains consistency with the rest of the codebase without affecting any functionality.
Line range hint
607-740: LGTM: Uniform formatting for Field subclasses and CadenceType classesAll Field subclasses (e.g.,
StructField,ResourceField,EventField, etc.) and CadenceType classes have been reformatted to use a consistent, modern Kotlin style. This improves code readability and maintains a uniform structure across the codebase without altering any functionality.sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (2)
69-72: LGTM: Improved readability with builder pattern.The use of the builder pattern for creating
Access.BlockHeaderResponseenhances code readability without altering functionality. This change aligns well with the PR's objective of improving code clarity.
84-87: LGTM: Consistent use of builder pattern improves readability.The changes throughout the file consistently apply the builder pattern for creating various response objects. This uniformly enhances code readability and maintainability without altering functionality. The multi-line format makes the object creation process clearer and easier to modify in the future.
Also applies to: 99-102, 113-116, 128-131, 143-146, 158-161, 172-175, 187-190, 203-204, 225-228, 245-248, 266-269, 286-289, 296-302, 310-313, 320-327, 335-338, 345-352, 359-369, 376-383, 390-400, 411-414, 425-428, 440-443, 455-460, 462-465, 480-489, 500-520, 535-545, 564-567, 612-615, 659-662, 706-709
sdk/src/main/kotlin/org/onflow/flow/sdk/transaction-dsl.kt (5)
48-51: LGTM: Improved function concisenessThe
simpleFlowTransactionfunction has been refactored into a more concise single-expression function. This change enhances readability while maintaining the same functionality, adhering to Kotlin's idiomatic style.
511-517: LGTM: Enhanced function readabilityThe
applyAsPayloadSignaturefunction has been refactored into a more concise single-expression function using awhenexpression. This change improves code readability while maintaining the same logic, which is in line with Kotlin's best practices.
536-542: LGTM: Consistent function refactoringThe
applyAsEnvelopeSignaturefunction has been refactored in the same manner asapplyAsPayloadSignature, converting it to a single-expression function using awhenexpression. This change maintains consistency in the codebase and improves readability.
563-568: LGTM: Simplified class structureThe
FlowArgumentsBuilderclass has been simplified:
- The private variable
_valueshas been renamed tovalues, removing the underscore prefix.- The
build()function has been converted to a more concise single-expression function.These changes improve code readability without altering the class's functionality.
572-574: LGTM: Consistent simplification across builder classesThe
FlowTransactionSignatureCollectionBuilderandFlowAddressCollectionBuilderclasses have been simplified in a manner consistent with theFlowArgumentsBuilder:
- In both classes, the private variable
_valueshas been renamed tovalues, removing the underscore prefix.- The
build()functions have been converted to more concise single-expression functions.These changes maintain consistency across the codebase and improve readability without altering the classes' functionality.
Also applies to: 602-602, 675-677, 682-682
sdk/src/main/kotlin/org/onflow/flow/sdk/impl/AsyncFlowAccessApiImpl.kt (17)
17-18: LGTM: Improved class declaration readabilityThe split of the class declaration improves readability by clearly separating the class name from its implemented interface.
76-81: LGTM: Consistent improvement in method call readabilityThe restructuring of the
api.getBlockHeaderByID()call is consistent with the previous changes, enhancing readability and maintainability.
105-110: LGTM: Consistent formatting improvementThe restructuring of the
api.getBlockHeaderByHeight()call maintains consistency with previous changes, further enhancing the overall readability of the code.
134-139: LGTM: Consistent formatting improvementThe restructuring of the
api.getLatestBlock()call continues the pattern of improving readability through consistent formatting across the file.
159-164: LGTM: Consistent formatting improvementThe restructuring of the
api.getBlockByID()call maintains the consistent formatting improvement pattern, enhancing overall code readability.
188-193: LGTM: Consistent formatting improvementThe restructuring of the
api.getBlockByHeight()call continues the consistent formatting improvement pattern, further enhancing the overall code readability.
217-222: LGTM: Consistent formatting improvementThe restructuring of the
api.getCollectionByID()call maintains the consistent formatting improvement pattern, contributing to the overall enhanced readability of the code.
246-251: LGTM: Consistent formatting improvementThe restructuring of the
api.sendTransaction()call continues the consistent formatting improvement pattern, further enhancing the overall code readability.
271-276: LGTM: Consistent formatting improvementThe restructuring of the
api.getTransaction()call maintains the consistent formatting improvement pattern, contributing to the enhanced overall readability of the code.
300-305: LGTM: Consistent formatting improvementThe restructuring of the
api.getTransactionResult()call continues the consistent formatting improvement pattern, further enhancing the overall code readability.
326-331: LGTM: Consistent formatting improvementThe restructuring of the
api.getAccount()call maintains the consistent formatting improvement pattern, contributing to the enhanced overall readability of the code.
355-360: LGTM: Consistent formatting improvementThe restructuring of the
api.getAccountAtLatestBlock()call continues the consistent formatting improvement pattern, further enhancing the overall code readability.
547-551: LGTM: Consistent formatting improvementThe restructuring of the
api.getNetworkParameters()call is consistent with the established pattern, improving readability by separating each part of the method call onto its own line.
571-575: LGTM: Consistent formatting improvementThe restructuring of the
api.getLatestProtocolStateSnapshot()call is consistent with the established pattern, enhancing readability by separating each part of the method call onto its own line.
596-599: LGTM: Consistent formatting improvementThe restructuring of the
api.getTransactionsByBlockID()call aligns with the established pattern, improving code readability by separating each part of the method call onto its own line.
621-624: LGTM: Consistent formatting improvementsThe restructuring of both
api.getTransactionResultsByBlockID()andapi.getExecutionResultByID()calls is consistent with the established pattern. These changes enhance code readability by separating each part of the method calls onto their own lines.Also applies to: 645-650
Line range hint
1-691: Overall assessment: Significant improvement in code readabilityThe changes made to this file consistently improve code readability by restructuring method calls, particularly
newBuilder()calls. This enhancement makes the code more maintainable and easier to modify in the future.A few instances (noted in previous comments) could benefit from further restructuring to maintain full consistency across the file. Addressing these minor inconsistencies would perfect the formatting improvements.
Great job on enhancing the code structure!
common/build.gradle.kts (2)
58-60: Integration Test Source Set Configuration Looks GoodThe
intTestsource set is correctly configured to include outputs frommainandtestFixtures, and the source directory is appropriately set.
22-24: Ensure Compatibility with JVM Target Version 21Setting the
jvmTargettoJVM_21updates the project to use Java 21. Please verify that all dependencies and environments support Java 21 to prevent any compatibility issues.Run the following script to check for any modules or files in the codebase that are targeting a different JVM version:
✅ Verification successful
JVM Target Version 21 Compatibility Verified
All
jvmTargetsettings are correctly set toJVM_21. No compatibility issues detected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all occurrences of 'jvmTarget' not set to JVM_21 # Search for 'jvmTarget' settings in Kotlin compiler options rg --type gradle -A 2 $'jvmTarget' | grep -v 'JVM_21'Length of output: 199
Script:
#!/bin/bash # Description: Find all occurrences of 'jvmTarget' not set to JVM_21 in .gradle.kts files # Search for 'jvmTarget' settings in Kotlin Gradle build scripts excluding those set to JVM_21 fd --extension gradle.kts --type file | xargs grep -H 'jvmTarget' | grep -v 'JVM_21'Length of output: 82
Script:
#!/bin/bash # Description: Comprehensive search for 'jvmTarget' not set to JVM_21 across all Gradle files # Search for 'jvmTarget' settings in any Gradle Kotlin scripts excluding those set to JVM_21 rg 'jvmTarget\s*=\s*["'\''"]?JVM_21["'\''"]?' --type-add gradle-kts=**/*.gradle.kts,**/*.gradle --files | xargs grep -H 'jvmTarget' | grep -v 'JVM_21'Length of output: 210
Script:
#!/bin/bash # Description: Comprehensive search for 'jvmTarget' not set to JVM_21 across all Gradle files with corrected type definitions # Define the new type 'gradle-kts' with the correct glob patterns rg --type-add 'gradle-kts:*.gradle.kts,*.gradle' 'jvmTarget\s*=\s*["'\''"]?JVM_21["'\''"]?' --type gradle-kts | grep -v 'JVM_21'Length of output: 188
build.gradle.kts (3)
15-17: Confirm Compatibility with Updated Plugin VersionsThe Kotlin JVM plugin, Dokka, and Kotlinter have been updated to versions "2.0.21", "2.0.0-Beta", and "4.4.1" respectively. Please verify that these new versions are compatible with your project's codebase and that any required adjustments due to deprecations or breaking changes have been made.
64-66: Ensure Environment Compatibility with JVM Target 21The
jvmTargethas been updated toJvmTarget.JVM_21, which targets Java 21. Verify that all development and deployment environments support Java 21 to prevent any runtime issues. If some environments do not support Java 21, consider maintaining compatibility with a lower JVM target.Also applies to: 76-78
71-71: Verify Compatibility ofkotlin-reflectDependencyThe
kotlin-reflectlibrary has been updated to version2.0.21. Ensure that all other Kotlin libraries and dependencies are also compatible with Kotlin2.0.21to avoid any potential mismatches or runtime errors.sdk/build.gradle.kts (4)
59-60: Check logging framework compatibility with SLF4J 2.xThe SLF4J API has been updated to version
2.0.16, and Logback Classic to1.5.9. SLF4J 2.x introduces changes that may affect logging implementations. Verify that all logging configurations and any custom logging code are compatible with these new versions.Review the logging configurations and update any deprecated settings or classes.
Line range hint
88-89: Ensure compatibility with Java 21 and Kotlin 2.0The project now targets Java 21, and the Kotlin version has been updated to 2.0. Ensure that:
- All dependencies and plugins are compatible with Java 21.
- The use of
compilerOptionswithJvmTarget.JVM_21is supported by the Kotlin Gradle plugin version in use.- No deprecated Java APIs are being used in the codebase.
Confirm that the Kotlin Gradle plugin is updated to support Kotlin 2.0 and the new
compilerOptionsDSL.Also applies to: 92-93
24-30: Ensure 'intTest' source set is properly configuredThe addition of the
intTestsource set for integration tests is a good practice. Verify that it is correctly configured and that the integration tests are being recognized and executed as intended.Run the following script to confirm that the
intTesttests are being detected:#!/bin/bash # Description: List all Kotlin test files in the 'intTest' source set. # Test: Find all test classes in 'src/intTest'. find src/intTest -name "*Test.kt"Additionally, ensure that the
integrationTesttask is properly set up to execute these tests.
33-53: Verify compatibility of updated dependenciesMultiple dependencies have been updated to newer major versions. These include:
- Kotlin Reflect updated to
2.0.21- Dokka plugin updated to
2.0.0-Beta- Kotlin Serialization JSON updated to
1.7.3- Jackson BOM updated to
2.18.0- Kotlin Coroutines updated to
1.9.0- JUnit Jupiter updated to
5.11.2- AssertJ Core updated to
3.26.3- Mockito Core and Inline updated to
5.x- SLF4J API updated to
2.0.16- Logback Classic updated to
1.5.9Please review the release notes for these dependencies to identify any breaking changes or required code modifications.
Run the following script to identify any deprecated APIs or breaking changes used in the codebase:
Ensure all usages are compatible with the updated dependencies.
✅ Verification successful
Dependencies are compatible
After searching the codebase, no usages of deprecated or changed coroutine APIs (
runBlocking,launch,async) were found. This confirms that the updated dependencies are compatible with the current code.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for deprecated or changed APIs in the codebase. # Test: Use 'rg' to find usages of known deprecated methods. rg --type kotlin 'runBlocking\(|launch\(|async\(' src/mainLength of output: 176
Script:
#!/bin/bash # Description: Search for deprecated or changed APIs in the Kotlin codebase. # Corrected search for deprecated coroutine APIs in src/main and src/test directories rg --type kotlin 'runBlocking\(|launch\(|async\(' src/main src/testLength of output: 167
Script:
#!/bin/bash # Description: Identify directories containing Kotlin source files. # List all directories in the repository echo "Listing all directories:" find . -type d # Locate all Kotlin files to determine source directories echo "Locating Kotlin files:" fd --extension ktLength of output: 20071
sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt
Outdated
Show resolved
Hide resolved
| val intTestImplementation: Configuration by configurations.creating { | ||
| extendsFrom(configurations["implementation"]) | ||
| } | ||
|
|
||
| val intTestRuntimeOnly: Configuration by configurations.creating { | ||
| extendsFrom(configurations["runtimeOnly"]) | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Integrate New Integration Test Configurations
You've added intTestImplementation and intTestRuntimeOnly configurations extending from implementation and runtimeOnly. Ensure that these configurations are properly utilized by defining the corresponding source sets and tasks for integration tests. This will enable the project to compile and run integration tests separately from unit tests.
Consider adding the following to define the integration test source set and tasks:
sourceSets {
val intTest by creating {
compileClasspath += sourceSets.main.get().output + configurations.intTestImplementation
runtimeClasspath += output + compileClasspath + configurations.intTestRuntimeOnly
}
}
tasks.register<Test>("intTest") {
description = "Runs the integration tests."
group = "verification"
testClassesDirs = sourceSets["intTest"].output.classesDirs
classpath = sourceSets["intTest"].runtimeClasspath
shouldRunAfter("test")
}| @@ -24,7 +25,7 @@ | |||
| mavenCentral() | |||
| } | |||
| dependencies { | |||
| classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.9.22") | |||
| classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21") | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove Redundant buildscript Block
Since the Kotlin Gradle plugin is already specified in the plugins block with the updated version, the buildscript block containing the classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21") dependency is redundant. Removing it can simplify your build script.
Apply this diff to remove the redundant buildscript block:
buildscript {
repositories {
google()
mavenCentral()
}
dependencies {
- classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21")
}
}Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Looking into this
There was a problem hiding this comment.
@lealobanov, sure, please let me know if you have any questions or need further assistance.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (15)
sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/composite/JsonCadenceBuilderEnumFieldTest.kt (1)
Line range hint
1-54: Consider adding more comprehensive testsThe test suite for
EnumFieldis well-structured and covers essential functionality. To further enhance the robustness of the tests, consider adding the following:
- Test with non-empty
CompositeValuearrays to ensure correct behavior with actual enum values.- Test the
decodeToAnymethod with more complex enum structures.- Add edge case tests, such as handling of null values or empty identifiers.
- Consider property-based testing to generate a wide range of inputs automatically.
These additions would provide more comprehensive coverage and help catch potential edge case bugs.
sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/composite/JsonCadenceBuilderStructFieldTest.kt (2)
Line range hint
18-26: LGTM! Consider adding a negative test case.The
Test hashCodemethod effectively verifies thatStructFieldobjects with equivalentCompositeValueobjects produce the same hash code. This is crucial for ensuring consistent behavior in hash-based collections.To further strengthen the test suite, consider adding a negative test case that verifies that
StructFieldobjects with differentCompositeValueobjects produce different hash codes. This would ensure that thehashCodemethod correctly differentiates between distinctStructFieldobjects.
Line range hint
28-41: LGTM! Consider adding edge cases.The
Test equalsmethod effectively verifies the equality and inequality ofStructFieldobjects based on their underlyingCompositeValueobjects. It covers both positive and negative cases, which is excellent.To make the test even more robust, consider adding the following edge cases:
- Compare a
StructFieldobject withnull.- Compare a
StructFieldobject with an object of a different type.- Test reflexivity by comparing a
StructFieldobject with itself.These additional cases would ensure that the
equalsmethod adheres to all the general contract requirements for theObject.equalsmethod in Java/Kotlin.kotlin-example/src/test/kotlin/org/onflow/examples/kotlin/createAccount/CreateAccountExampleTest.kt (1)
Line range hint
65-69: Enhance the account balance test for better coverage.While the new test method is a good addition, consider the following improvements to make it more robust:
- Test for a specific balance value instead of just checking for non-null.
- Add error handling to test for potential exceptions.
- Consider testing with different account states (e.g., newly created account, account with transactions).
Here's a suggested improvement:
@Test fun `Can get an account balance`() { // Test with service account val serviceBalance = accessAPIConnector.getAccountBalance(serviceAccount.flowAddress) Assertions.assertNotNull(serviceBalance) Assertions.assertTrue(serviceBalance > BigDecimal.ZERO) // Test with a newly created account val newAccountKey = userKeyPairs[0].public val newAccountAddress = createUserAccount(newAccountKey) val newAccountBalance = accessAPIConnector.getAccountBalance(newAccountAddress) Assertions.assertEquals(BigDecimal.ZERO, newAccountBalance) // Test error handling Assertions.assertThrows(IllegalArgumentException::class.java) { accessAPIConnector.getAccountBalance(FlowAddress("0x1234567890abcdef")) } }This enhanced version tests multiple scenarios and includes error handling, providing better coverage of the
getAccountBalancefunctionality.sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (4)
76-79: LGTM! Consider adding a brief comment for clarity.The changes to
decodeJsonCadenceListlook good. The new byte array method provides more flexibility, and the string method now uses it for consistency.Consider adding a brief comment to explain the purpose of these methods:
/** * Decodes a JSON Cadence list from a string or byte array. * @param string The JSON string to decode * @return A list of decoded Cadence fields */ @JvmStatic fun <T : Field<*>> decodeJsonCadenceList(string: String): List<T> = decodeJsonCadenceList(string.toByteArray(Charsets.UTF_8)) /** * Decodes a JSON Cadence list from a byte array. * @param bytes The JSON byte array to decode * @return A list of decoded Cadence fields */ @JvmStatic fun <T : Field<*>> decodeJsonCadenceList(bytes: ByteArray): List<T> = OBJECT_MAPPER.readValue(bytes, object : TypeReference<List<T>>() {})
82-85: LGTM! Consider adding a brief comment for clarity.The changes to
decodeJsonCadenceare consistent with the modifications made todecodeJsonCadenceList. The new byte array method provides more flexibility, and the string method now uses it for consistency.Consider adding a brief comment to explain the purpose of these methods:
/** * Decodes a single JSON Cadence object from a string. * @param string The JSON string to decode * @return A decoded Cadence field */ @JvmStatic fun <T : Field<*>> decodeJsonCadence(string: String): T = decodeJsonCadence(string.toByteArray(Charsets.UTF_8)) /** * Decodes a single JSON Cadence object from a byte array. * @param bytes The JSON byte array to decode * @return A decoded Cadence field */ @JvmStatic fun <T : Field<*>> decodeJsonCadence(bytes: ByteArray): T = OBJECT_MAPPER.readValue(bytes, object : TypeReference<T>() {})
88-89: LGTM! Consider adding a brief comment for clarity.The addition of
encodeJsonCadenceListcomplements the decoding methods, providing a complete API for JSON Cadence data handling. The implementation is consistent with the rest of the class.Consider adding a brief comment to explain the purpose of this method:
/** * Encodes a list of JSON Cadence objects to a byte array. * @param jsonCadences The list of Cadence fields to encode * @return A byte array representation of the encoded JSON */ @JvmStatic fun <T : Field<*>> encodeJsonCadenceList(jsonCadences: Iterable<T>): ByteArray = OBJECT_MAPPER.writeValueAsBytes(jsonCadences)
Line range hint
91-92: LGTM! Consider adding a brief comment for clarity.The addition of
encodeJsonCadencecomplements the decoding methods, providing a complete API for JSON Cadence data handling. The implementation is consistent with the rest of the class.Consider adding a brief comment to explain the purpose of this method:
/** * Encodes a single JSON Cadence object to a byte array. * @param jsonCadence The Cadence field to encode * @return A byte array representation of the encoded JSON */ @JvmStatic fun <T : Field<*>> encodeJsonCadence(jsonCadence: T): ByteArray = OBJECT_MAPPER.writeValueAsBytes(jsonCadence)sdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionDecodingTest.kt (1)
36-39: Improved readability of FlowTransactionSignature instantiation.The reformatting of the
FlowTransactionSignatureparameters enhances code readability without altering the test's functionality. This change aligns with Kotlin style guidelines for complex method calls.Consider applying similar formatting to other complex instantiations in the file for consistency.
kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/streaming/streamEvents/SubscribeEventsExample.kt (4)
Line range hint
30-39: Handle unexpected exceptions indataJob.Currently, only
CancellationExceptionis caught. If any other exceptions occur within the coroutine, they may cause it to fail silently. Consider catching all exceptions to ensure that unexpected errors are properly logged and handled.Apply this diff to handle all exceptions:
try { for (events in dataChannel) { if (!isActive) break if (events.isNotEmpty()) { receivedEvents.addAll(events) } yield() - } catch (e: CancellationException) { - println("Data channel processing cancelled") + } catch (e: CancellationException) { + println("Data channel processing cancelled") + } catch (e: Exception) { + println("Exception in data channel processing: ${e.message}") } finally {
Line range hint
44-53: Handle unexpected exceptions inerrorJob.Similar to
dataJob,errorJobonly catchesCancellationException. To prevent the coroutine from failing silently due to unexpected exceptions, consider catching all exceptions.Apply this diff:
try { for (error in errorChannel) { println("~~~ ERROR: ${error.message} ~~~") if (!isActive) break yield() - } catch (e: CancellationException) { - println("Error channel processing cancelled") + } catch (e: CancellationException) { + println("Error channel processing cancelled") + } catch (e: Exception) { + println("Exception in error channel processing: ${e.message}") } finally {
Line range hint
36-36: Consider using a logging framework instead ofprintln.Using
printlnstatements is not recommended for production code. A logging framework provides better control over logging levels and output formats.Also applies to: 50-50
Line range hint
32-32: RedundantisActivechecks inside the coroutines.The
isActivecheck inside theforloops may be unnecessary because coroutines automatically handle cancellation during suspension points like channel iteration. Removing the checks can simplify the code.Apply this diff to remove the redundant checks in
dataJob:try { for (events in dataChannel) { - if (!isActive) break if (events.isNotEmpty()) { receivedEvents.addAll(events) } yield()And in
errorJob:try { for (error in errorChannel) { println("~~~ ERROR: ${error.message} ~~~") - if (!isActive) break yield()Also applies to: 47-47
build.gradle.kts (1)
72-72: Typographical error in commented-out codeThere's a misplaced quotation mark in the commented-out line, which might cause confusion. If you plan to use this line later, consider correcting it.
Apply this diff to fix the typographical error:
-//dokkaHtmlPlugin"("org.jetbrains.dokka:kotlin-as-java-plugin:2.0.0-Beta") +// dokkaHtmlPlugin("org.jetbrains.dokka:kotlin-as-java-plugin:2.0.0-Beta")sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (1)
Line range hint
473-491: Add error handling for decoding and index accessIn the
of(bytes: ByteArray)method, potential exceptions may occur during decoding or when accessingtx.signerList[it.signerIndex]. To enhance robustness, consider adding error handling to manage exceptions fromRLPCodec.decodeand validate indices when accessing lists to preventIndexOutOfBoundsException.Here's an example of how you might add error handling:
@JvmStatic fun of(bytes: ByteArray): FlowTransaction { val txEnvelope: TransactionEnvelope = try { RLPCodec.decode(bytes, TransactionEnvelope::class.java) } catch (e: Exception) { throw FlowException("Failed to decode transaction envelope", e) } var tx = FlowTransaction( script = FlowScript(txEnvelope.payload.script), arguments = txEnvelope.payload.arguments.map { FlowArgument(it) }, referenceBlockId = FlowId.of(txEnvelope.payload.referenceBlockId), gasLimit = txEnvelope.payload.gasLimit, proposalKey = FlowTransactionProposalKey( FlowAddress.of(txEnvelope.payload.proposalKeyAddress), txEnvelope.payload.proposalKeyIndex.toInt(), txEnvelope.payload.proposalKeySequenceNumber ), payerAddress = FlowAddress.of(txEnvelope.payload.payer), authorizers = txEnvelope.payload.authorizers.map { FlowAddress.of(it) } ) txEnvelope.payloadSignatures.forEach { if (it.signerIndex !in tx.signerList.indices) { throw FlowException("Invalid signerIndex ${it.signerIndex} in payloadSignatures") } tx = tx.addPayloadSignature(tx.signerList[it.signerIndex], it.keyIndex, FlowSignature(it.signature)) } txEnvelope.envelopeSignatures.forEach { if (it.signerIndex !in tx.signerList.indices) { throw FlowException("Invalid signerIndex ${it.signerIndex} in envelopeSignatures") } tx = tx.addEnvelopeSignature(tx.signerList[it.signerIndex], it.keyIndex, FlowSignature(it.signature)) } return tx }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
- .editorconfig (0 hunks)
- build.gradle.kts (4 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/streaming/streamEvents/SubscribeEventsExample.kt (1 hunks)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/verifySignature/userSignature/UserSignatureExample.kt (1 hunks)
- kotlin-example/src/test/kotlin/org/onflow/examples/kotlin/createAccount/CreateAccountExampleTest.kt (1 hunks)
- sdk/build.gradle.kts (2 hunks)
- sdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionDecodingTest.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (1 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/cadence/json-cadence.kt (14 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/errors.kt (3 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (3 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/transaction-dsl.kt (9 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/composite/JsonCadenceBuilderEnumFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/composite/JsonCadenceBuilderStructFieldTest.kt (1 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/crypto/SignTest.kt (1 hunks)
💤 Files with no reviewable changes (1)
- .editorconfig
✅ Files skipped from review due to trivial changes (2)
- kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/verifySignature/userSignature/UserSignatureExample.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/crypto/SignTest.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- sdk/src/main/kotlin/org/onflow/flow/sdk/cadence/json-cadence.kt
- sdk/src/main/kotlin/org/onflow/flow/sdk/errors.kt
- sdk/src/main/kotlin/org/onflow/flow/sdk/transaction-dsl.kt
🧰 Additional context used
🔇 Additional comments (13)
sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/composite/JsonCadenceBuilderEnumFieldTest.kt (1)
Line range hint
17-27: LGTM: Good addition of hashCode testThe new
Test hashCodemethod is a valuable addition to the test suite. It correctly verifies thatEnumFieldinstances with the same content produce the same hash code, which is crucial for maintaining the contract betweenequals()andhashCode(). This test complements the existingTest equalsmethod, ensuring consistent behavior whenEnumFieldobjects are used in hash-based collections.sdk/src/test/kotlin/org/onflow/flow/sdk/cadence/fields/composite/JsonCadenceBuilderStructFieldTest.kt (1)
Line range hint
17-41: Great additions to the test suite!The new
Test hashCodeandTest equalsmethods significantly enhance the test coverage for theStructFieldclass. These tests verify crucial properties:
- Consistent hash codes for equivalent
StructFieldobjects.- Correct equality comparisons between
StructFieldobjects.These additions improve the overall quality and reliability of the test suite, helping to ensure that the
StructFieldclass behaves correctly in various scenarios, particularly when used in collections or compared for equality.The suggested minor improvements (adding negative test cases for
hashCodeand edge cases forequals) would further strengthen the tests, but the current implementation already provides solid coverage.kotlin-example/src/test/kotlin/org/onflow/examples/kotlin/createAccount/CreateAccountExampleTest.kt (1)
Line range hint
65-69: Good integration of the new test method.The new
Can get an account balancetest method is well-integrated into the existing test class structure. It complements the other account-related tests and follows the established patterns in the class. The use ofaccessAPIConnectoris consistent with other tests, ensuring proper setup and teardown.sdk/src/main/kotlin/org/onflow/flow/sdk/Flow.kt (1)
Line range hint
76-92: Overall, excellent additions to enhance JSON Cadence data handling!The new methods for encoding and decoding JSON Cadence data using byte arrays are well-implemented and consistent with the existing codebase. These additions provide more flexibility for users and complete the API for JSON Cadence data handling.
Key improvements:
- Consistent use of byte arrays as the underlying data format.
- New methods complement existing string-based methods.
- Implementation uses the
OBJECT_MAPPERconsistently across all methods.The suggested documentation improvements will further enhance the usability of these methods.
sdk/src/intTest/org/onflow/flow/sdk/transaction/TransactionDecodingTest.kt (1)
Line range hint
1-105: Summary: Formatting improvements align with PR objectives.The changes in this file focus on improving code readability, particularly in the
Can decode transaction envelopetest method. These modifications align well with the PR objectives of addressing outstanding lint issues and enhancing code quality. No functional changes were introduced, which is appropriate for a dependency update PR.kotlin-example/src/main/kotlin/org/onflow/examples/kotlin/streaming/streamEvents/SubscribeEventsExample.kt (2)
Line range hint
26-58: Great job refactoring event processing intoprocessEventsmethod!This change enhances readability and maintainability by separating concerns and encapsulating the event processing logic.
Line range hint
40-40: Review the necessity of cancelling channels infinallyblocks.Cancelling
dataChannelanderrorChannelin thefinallyblocks may not be necessary if the channels are closed by the producer or when they are no longer needed. Ensure that cancelling them here does not interfere with other parts of the code that may be using these channels.Also applies to: 55-55
build.gradle.kts (4)
15-15: Verify compatibility with Kotlin 2.0.21The project's Kotlin plugin version has been updated to
2.0.21. Please ensure that all project dependencies, plugins, and code are compatible with Kotlin 2.0, as there may be breaking changes from previous versions.
17-17: Update Ktlint plugin to 4.4.1The Ktlint plugin version has been updated to
4.4.1. Ensure that your.editorconfigand any custom lint rules are updated to align with this new version.
64-66: Verify compatibility with JVM target 21You've updated the JVM target to Java 21 (
JvmTarget.JVM_21). Please verify that all dependencies and the deployment environment support Java 21, as some libraries may not yet be compatible with this version.
71-71: Updatekotlin-reflectdependency to 2.0.21The
kotlin-reflectdependency has been updated to version2.0.21, matching the Kotlin plugin version. This alignment helps prevent version inconsistencies.sdk/build.gradle.kts (1)
32-62: Ensure compatibility with updated dependenciesMajor version updates for several dependencies may introduce breaking changes or deprecations. Specifically:
- Kotlin Coroutines upgraded from
1.5.2to1.9.0- SLF4J upgraded from
1.7.30to2.0.16- Mockito upgraded from
3.xto5.x- Jackson BOM upgraded from
2.16.1to2.18.0Please verify that the codebase is compatible with these new versions and update any necessary code to address any breaking changes or deprecated APIs.
You can run the following script to identify potential issues related to deprecated or changed APIs:
sdk/src/main/kotlin/org/onflow/flow/sdk/models.kt (1)
Line range hint
613-617: Verify correct assignment ofsignerIndexIn the
ofmethod ofFlowTransactionSignature, bothsignerIndexandkeyIndexare assignedvalue.keyId. Please verify ifsignerIndexshould be assignedvalue.signerIndexto accurately reflect the signer's index.If
valuecontains asignerIndexfield, consider updating the assignment:companion object { @JvmStatic fun of(value: TransactionOuterClass.Transaction.Signature): FlowTransactionSignature = FlowTransactionSignature( address = FlowAddress.of(value.address.toByteArray()), - signerIndex = value.keyId, + signerIndex = value.signerIndex, keyIndex = value.keyId, signature = FlowSignature(value.signature.toByteArray()) ) }
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
sdk/build.gradle.kts (1)
24-30: Consider renaming the integration test source set for clarityThe source set
intTestcould be renamed tointegrationTestto follow standard naming conventions in Gradle projects. This enhances readability and maintains consistency with common practices.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- common/build.gradle.kts (2 hunks)
- kotlin-example/build.gradle.kts (3 hunks)
- sdk/build.gradle.kts (2 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (31 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- kotlin-example/build.gradle.kts
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt
🧰 Additional context used
🔇 Additional comments (7)
common/build.gradle.kts (5)
22-24: LGTM: Kotlin compiler options updated correctlyThe changes to the Kotlin compiler options are appropriate:
- Using
compilerOptionsblock is the correct modern syntax.- Setting JVM target to 21 aligns with the PR objective of upgrading to the latest major release.
- Using
addAll()for compiler arguments is more idiomatic Kotlin.These updates improve the build configuration and keep it current with the latest Kotlin practices.
47-49: LGTM: Integration test dependencies added correctlyThe addition of integration test dependencies is well-structured:
- Separate
intTestImplementationconfiguration is used, which is a good practice.- Up-to-date versions of JUnit Jupiter (5.11.2) and AssertJ (3.26.3) are used.
- Including
junit-platform-launcheras a runtime dependency is appropriate for test execution.These changes improve the project's testing capabilities and align with the PR objective of updating dependencies.
58-60: LGTM: Integration test source set configured correctlyThe changes to the
intTestsource set configuration are well-structured:
- Including both
mainandtestFixturesoutput in the classpath ensures integration tests have access to all necessary classes.- Specifying the Kotlin source directory for integration tests (
src/intTest/kotlin) improves project organization.These updates enhance the integration testing setup and maintain a clear separation of concerns between different test types.
38-40:⚠️ Potential issueResolve inconsistent Mockito versions
The Mockito dependencies have inconsistent versions:
mockito-coreis version 5.14.1mockito-inlineis version 5.2.0This inconsistency was previously flagged and should be resolved to prevent potential conflicts.
Please update the
mockito-inlineversion to matchmockito-core:testImplementation("org.junit.jupiter:junit-jupiter:5.11.2") testImplementation("org.mockito:mockito-core:5.14.1") -testImplementation("org.mockito:mockito-inline:5.2.0") +testImplementation("org.mockito:mockito-inline:5.14.1")
42-45:⚠️ Potential issueResolve inconsistent Mockito versions in test fixtures
The same Mockito version inconsistency exists in the test fixtures dependencies:
mockito-coreis version 5.14.1mockito-inlineis version 5.2.0This issue was previously flagged and should be resolved for consistency across all test configurations.
Please update the
mockito-inlineversion in test fixtures to matchmockito-core:testFixturesImplementation("org.junit.jupiter:junit-jupiter:5.11.2") testFixturesImplementation("org.mockito:mockito-core:5.14.1") -testFixturesImplementation("org.mockito:mockito-inline:5.2.0") +testFixturesImplementation("org.mockito:mockito-inline:5.14.1")sdk/build.gradle.kts (2)
33-60: Ensure compatibility with updated dependenciesSeveral dependencies have been upgraded to new major versions, including Kotlin Reflect, Kotlinx Serialization, Jackson BOM, Coroutines, JUnit, Mockito, SLF4J, and Logback. These major version upgrades may introduce breaking changes or deprecations. Please verify that the codebase is compatible with these new versions and update any code that may be affected.
92-93: Previous suggestion still applicableThe earlier recommendation to simplify the
jvmTargetconfiguration remains valid. Please consider implementing the suggested changes to enhance code readability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
build.gradle.kts (2)
15-17: LGTM! Consider updating the changelog.The updates to Kotlin (2.0.21) and Kotlinter (4.4.1) align with the PR objectives of upgrading to the latest major Kotlin release and updating dependencies. These changes should improve compatibility with the latest Kotlin features and best practices.
Don't forget to update the project's changelog to reflect these significant version upgrades, especially the major Kotlin version change.
64-66: LGTM! Consider removing the line number comment.The updates to the Kotlin compile task are correct and align with Kotlin 2.0 syntax. Setting the JVM target to 21 and adding compiler arguments for strict null checks and opt-in requirements are good practices for maintaining code quality.
Consider removing the commented-out line number
// 65as it's not necessary and may become outdated if the file structure changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- build.gradle.kts (4 hunks)
- common/build.gradle.kts (3 hunks)
- sdk/build.gradle.kts (3 hunks)
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt (20 hunks)
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt (29 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- common/build.gradle.kts
- sdk/src/main/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImpl.kt
- sdk/src/test/kotlin/org/onflow/flow/sdk/impl/FlowAccessApiImplTest.kt
🧰 Additional context used
🔇 Additional comments (8)
build.gradle.kts (5)
28-28: Remove redundant buildscript blockThe
buildscriptblock containing the Kotlin Gradle plugin classpath is redundant since the Kotlin plugin is already specified in thepluginsblock with the updated version.As previously suggested, consider removing the entire
buildscriptblock to simplify your build script:-buildscript { - repositories { - google() - mavenCentral() - } - dependencies { - classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:2.0.21") - } -}
55-62: LGTM! Consider fully integrating the new test configurations.The addition of
intTestImplementationandintTestRuntimeOnlyconfigurations is a good step towards setting up integration tests.As previously suggested, to fully utilize these configurations, consider defining the corresponding source sets and tasks for integration tests. This will enable the project to compile and run integration tests separately from unit tests. Here's a reminder of the suggested additions:
sourceSets { val intTest by creating { compileClasspath += sourceSets.main.get().output + configurations.intTestImplementation runtimeClasspath += output + compileClasspath + configurations.intTestRuntimeOnly } } tasks.register<Test>("intTest") { description = "Runs the integration tests." group = "verification" testClassesDirs = sourceSets["intTest"].output.classesDirs classpath = sourceSets["intTest"].runtimeClasspath shouldRunAfter("test") }
71-71: LGTM! Kotlin reflect dependency updated.The update of the Kotlin reflect dependency to version 2.0.21 is consistent with the Kotlin plugin update and ensures compatibility.
76-78: LGTM! Test compilation options updated.The updates to the test Kotlin compile task are consistent with the main compile task changes. This ensures that both main and test code are compiled with the same options, maintaining consistency across the project.
Line range hint
1-97: Overall, the changes look good with some points to addressThe updates to the
build.gradle.ktsfile generally align with the PR objectives of upgrading to the latest major Kotlin release and updating dependencies. The changes improve the project's build configuration and align with modern Kotlin practices. Here are the main points to address:
- Update the project's changelog to reflect the significant version upgrades.
- Consider removing the redundant
buildscriptblock.- Fully integrate the new integration test configurations by adding source sets and tasks.
- Clarify the status of the Dokka migration, as there's a discrepancy between the PR objectives and the commented-out dependency.
- Consider removing unnecessary comments, such as the line number comment in the compiler options.
Once these points are addressed, the build file will be in excellent shape for the dependency upgrades.
sdk/build.gradle.kts (3)
25-31: Integration test source set configured correctlyThe new
intTestsource set for integration tests is well-defined, with appropriate classpath settings and source directories. This enhances the project's testing structure by separating integration tests from unit tests.
33-65: Dependencies updated to latest versionsUpdating the dependencies to their latest versions keeps the project current with recent improvements, features, and security patches. Ensure thorough testing is conducted to verify compatibility with these new versions.
93-94: Compiler options updated to useJvmTargetThe
compilerOptionsblock correctly sets the JVM target to Java 21 usingJvmTarget.JVM_21. This aligns with Kotlin DSL practices and improves code readability.
| @@ -1,6 +1,6 @@ | |||
| distributionBase=GRADLE_USER_HOME | |||
| distributionPath=wrapper/dists | |||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-bin.zip | |||
| distributionUrl=https\://services.gradle.org/distributions/gradle-8.8-bin.zip | |||
There was a problem hiding this comment.
Downgraded to 8.8 as 8.9 is not officially supported by Kotlin 2.0 (or versions <2)
Description
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes