Skip to content

Response processing refactor#554

Merged
MiSikora merged 3 commits into
developfrom
response-processing-refactor
Feb 10, 2021
Merged

Response processing refactor#554
MiSikora merged 3 commits into
developfrom
response-processing-refactor

Conversation

@MiSikora

@MiSikora MiSikora commented Feb 9, 2021

Copy link
Copy Markdown
Contributor

📄 Context

This is the final PR that is a result of splitting #527.

📝 Changes

Similarly to the RequestProcessor class I added the ResponseProcessor class that does the analogous stuff. I changed one thing, where plain text body is set to be plainText = true only when there is some content. I don't think it made sense to set it to true when there was no body.

📎 Related PR

#527

🚫 Breaking

No.

🛠️ How to test

Rely on CI. Manually you can simply check the sample app.

⏱️ Next steps

N/A

Michał Sikora added 2 commits February 9, 2021 19:25
@cortinico cortinico added this to the 4.0.0 milestone Feb 9, 2021

@cortinico cortinico left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just a minor comment

Comment thread library/src/main/java/com/chuckerteam/chucker/api/ChuckerInterceptor.kt Outdated
@MiSikora

MiSikora commented Feb 9, 2021

Copy link
Copy Markdown
Contributor Author

Bumping because I don't want to get this message lost in a resolved comment. :)

@cortinico Ok, I pushed the change in 007cd55 without the init block. Let me know which one you prefer more. I'm ok either way.

@cortinico

Copy link
Copy Markdown
Member

@cortinico Ok, I pushed the change in 007cd55 without the init block. Let me know which one you prefer more. I'm ok either way.

I'd go for the solution without the init{} block as it's more declarative and easier to follow :)

@MiSikora MiSikora merged commit 5420645 into develop Feb 10, 2021
@MiSikora MiSikora deleted the response-processing-refactor branch February 10, 2021 15:58
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