Add export function to API#784
Conversation
There was a problem hiding this comment.
Please don't start a thread explicity here. We use coroutines to do any async work.
There was a problem hiding this comment.
Have updated to use coroutines, but have had to add coroutines and activity-ktx dependencies to the sample gradle project as they weren't there before
There was a problem hiding this comment.
Yeah I realized this is a side effect 🤔 I'll be fine with it. I'm unsure what's the other maintainers take on this (specifically @MiSikora )
Just noticed this. I would say that we can safely make this a suspending function at this stage. Moreover, tests are missing. Could you add some of them? |
|
I think I have covered all your comments. Could you give some more information about what functions you are expecting to be tested? The old |
|
@cortinico Thanks for looking over it. I think I have addressed all the comments above, but I'm not 100% sure about the testing side of it. Could you clarify what parts you think should have unit test coverage and I can look into it? |
|
@cortinico Can you give any advice about the above? |
3c35dfe to
fc561b1
Compare
Sorry this fell off @jd565 my bad. I was quite busy recently. I believe the best approach here would be to make I'd like to get your opinion on the last commit. If you're fine with it, we can merge it. |
vbuberen
left a comment
There was a problem hiding this comment.
The approach looks fine to me.
Have left a few comments about strange new imports.
Also, I feel that we need to mention this feature in our Readme file as well.
It seems like we haven't had the detekt rule enabled about unused imports. I've enabled it and fixed other issues in the codebase. |
|
@cortinico I also forgot about this :/ Your changes look fine and if you are happy to merge then I'm happy to. |
|
@jd565 @cortinico cound it be possible to add HAR file export too? |
Add an export function that writes transaction data to a file and returns a URI to access that file. The output format is the same as when exporting from the chucker UI, but can be triggered from code.
0ab10b3 to
bd55f0d
Compare
Yup I'm merging this one 👍 Thank you again @jd565
@BluestormDNA please open a separate issue |
Add an export function that writes transaction data to a file and
returns a URI to access that file. The output format is the same as when
exporting from the chucker UI, but can be triggered from code.
📷 Screenshots
All internal changes - nothing different in UI
📄 Context
Adds functionality to export transactions to a file on the API. Related issue
📝 Changes
Updated the Chucker API object to add in the new functionality. I've not made this a suspending function to maintain interop with java, so have just left a comment that this needs to be called on a background thread.
I added a button to the sample app (I had to comment out the strict mode checking otherwise the sample app closed immediately) which calls the new method - I'm not sure if this should be left in the sample long term so happy to remove it if it doesn't make sense.
📎 Related PR
🚫 Breaking
No
🛠️ How to test
Added a button to the sample app to export to a file then pulled the file and verified the contents.
⏱️ Next steps