Skip to content

chore: remove all logic for older version of plugin (latestVersion)#10

Merged
alexgerardojacinto merged 3 commits intofeat/RMET-4099/android-implementationfrom
feat/RMET-5076/remove-old-code
Mar 25, 2026
Merged

chore: remove all logic for older version of plugin (latestVersion)#10
alexgerardojacinto merged 3 commits intofeat/RMET-4099/android-implementationfrom
feat/RMET-5076/remove-old-code

Conversation

@alexgerardojacinto
Copy link
Contributor

@alexgerardojacinto alexgerardojacinto commented Mar 24, 2026

Source files:

  • IONCAMRCameraParameters.kt — removed latestVersion field
  • IONCAMRMediaProcessor.kt — processCameraImage: removed onImage and intent parameters, removed var bitmap: Bitmap?, collapsed the if/else to keep only the latestVersion = true branch
  • IONCAMRCameraManager.kt — processResultFromCamera: removed onImage parameter, updated the processCameraImage call to match the new signature

Test files:

  • TakePictureTests.kt — removed all legacy (latestVersion = false) tests; kept and updated the 4 givenAPI30 tests (removed latestVersion from constructor, removed onImage parameter from
    calls) and added a few new tests that were missing
  • ChoosePictureTests.kt — removed latestVersion = false from all IONCAMRCameraParameters constructors
  • EditPictureTests.kt — removed the two processResultFromCamera tests that tested the legacy code path (givenTakePictureAllowEditWhenEditProcess*), removed now-unused imports
    (IONCAMRCameraManager, IONCAMRCameraParameters) and the PROCESS_SUCCESS constant

@OS-pedrogustavobilro OS-pedrogustavobilro self-assigned this Mar 25, 2026
@Test
fun givenTakePictureNotCalledJPEGWhenProcessResultFromCameraThenTakePhotoError() {

fun givenAPI30TakePictureCalledJPEGAndDataUriAndUriNullWhenProcessResultFromCameraThenError() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious: did you find why some tests are called givenAPI30 and others givenAPI28? Was it just that the latter being called with latestVersion=false? It's kind of weird that they're labelled with the API level when that does not get into account for the actual test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good question, I don't really know why 😅 New tests were added by Claude and I think it did that because older tests already had that naming pattern?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, that's fair, I was asking more to know if you knew why they had the API level in the name before. But anyway it's non-blocker, just a curiosity, all good on my end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@OS-pedrogustavobilro I renamed the tests to remove that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But yeah I think originally we were setting the API level and that's why tests were named like that

@alexgerardojacinto alexgerardojacinto merged commit 98a5359 into feat/RMET-4099/android-implementation Mar 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants