Skip to content

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented May 27, 2024

Builds on #114 and introduces WpRequestBuilder as a replacement for WpApiHelper. The usage becomes:

// Before (from Kotlin)
apiHelper.retrieveUserRequest(userId, context)
// After (from Kotlin)
requestBuilder.users().retrieve(userId, context)

I was initially going to use the WpApi name as a general encapsulation, but while updating the Swift implementation, I realized it'd create a clash. Also, I think WpRequestBuilder is a more suitable name if we want to just keep it for request building. I still don't like the Wp prefix, but for now, this is consistent with rest of the naming.

Note that it's using Wp instead of WP as the prefix, because the bindings are generated with this prefix and the Rust layer will be updated to use it, so it's consistent everywhere. I didn't want to use WP just to change it in a later PR.

I've also updated the Current suffix for users with Me. This is consistent with the API endpoint and should make it easier to look up documentation if it's needed. I should have gone with this from the start and only didn't do so because I felt weird using Me.

To Test

  • make test-server && make dump-mysql && make backup-wp-content-plugins
  • cargo test --test '*' -- --nocapture --test-threads 1
  • cd native/kotlin && ./gradlew :api:kotlin:integrationTest cAT

@oguzkocer oguzkocer force-pushed the wp-api-request-builder branch from 4397652 to c051617 Compare May 27, 2024 18:46
@oguzkocer oguzkocer marked this pull request as ready for review May 27, 2024 18:56
@oguzkocer oguzkocer requested review from crazytonyli and jkmassel May 27, 2024 18:56
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No memory leaks related to this code, and the changes look straightforward.

@oguzkocer oguzkocer merged commit eada803 into trunk May 27, 2024
@oguzkocer oguzkocer deleted the wp-api-request-builder branch May 27, 2024 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants