Skip to content

Use builder pattern for ChuckerInterceptor in a backwards compatible way#491

Merged
MiSikora merged 1 commit into
developfrom
builder-pattern-3.x
Nov 5, 2020
Merged

Use builder pattern for ChuckerInterceptor in a backwards compatible way#491
MiSikora merged 1 commit into
developfrom
builder-pattern-3.x

Conversation

@MiSikora

@MiSikora MiSikora commented Nov 3, 2020

Copy link
Copy Markdown
Contributor

📄 Context

It is a follow up to the discussion in #483. It also addresses #462 and #466.

📝 Changes

I added a builder pattern in a 3.x compatible way. This way users will be informed of changes and have more time to adjust their code. Unfortunately, ReplaceWith is not ideal. imports do no work properly with default arguments but IMHO it is a minor inconvenience (basically, only ChuckerCollector needs to be manually imported). Also, it will create builder with all fields. I've tried to split constructor into different ones with different ReplaceWith strategies but it is not possible due to named arguments and source compatibility in Kotlin.

📎 Related PR

#462

🚫 Breaking

No.

🛠️ How to test

You can check if ReplaceWith works correctly.

⏱️ Next steps

Prepare 3.4.0 release and 3.x branch.

@MiSikora

MiSikora commented Nov 3, 2020

Copy link
Copy Markdown
Contributor Author

@cortinico

I'm not sure if that is what you wanted in here. Should I create a separate branch with just 2ead7cb cherry-picked or bringing all other commits is fine (I think these commits were supposed to land in 3.x anyway)? If yes, which merging strategy should I use?

@vbuberen vbuberen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You are right, we still need to merge other commits into 3.x as well as a part of 3.4.0 update. As to the strategy I believe that you should use Rebase and merge here to save the history.

With different PRs we will make branches develop and 3.x different anyway and it will add complexity with next merges into 3.x in future. I assume we will have to do 2 PRs (into develop and into 3.x) in future or play around with cherry-picking if something needs to land in both branches.

@MiSikora MiSikora added this to the 3.4.0 milestone Nov 4, 2020
@MiSikora MiSikora changed the base branch from 3.x to develop November 4, 2020 18:09
@MiSikora

MiSikora commented Nov 4, 2020

Copy link
Copy Markdown
Contributor Author

Changed branch target in order to make 3.x easier as we discussed on Slack.

@MiSikora MiSikora added the enhancement New feature or improvement to the library label Nov 4, 2020

@vbuberen vbuberen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tested - ReplaceWith works fine.

@MiSikora MiSikora merged commit 2ca618b into develop Nov 5, 2020
@MiSikora MiSikora deleted the builder-pattern-3.x branch November 5, 2020 13:45
@vbuberen vbuberen mentioned this pull request Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or improvement to the library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants