Skip to content

Added convenience overload for AddScreenDiff#231

Closed
CraigRichards wants to merge 1 commit into
allure-framework:mainfrom
CraigRichards:feature/AddScreenDiff-convenience-method
Closed

Added convenience overload for AddScreenDiff#231
CraigRichards wants to merge 1 commit into
allure-framework:mainfrom
CraigRichards:feature/AddScreenDiff-convenience-method

Conversation

@CraigRichards

@CraigRichards CraigRichards commented Jun 9, 2021

Copy link
Copy Markdown
Contributor

//: # (
. Thank you so much for sending us a pull request!
.
. Make sure you have a clear name for your pull request.
. The name should start with a capital letter and no dot is required in the end of the sentence.
. To link the request with isses use the following notation: (fixes #123, fixes #321)
.
. An example of good pull request names:
. - Add Russian translation (fixes #123)
. - Add an ability to disable default plugins
. - Support emoji in test descriptions
)

Context

Checklist

@CLAassistant

CLAassistant commented Jun 9, 2021

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@Bakanych

Copy link
Copy Markdown
Contributor

Hi @CraigRichards,
Idea is good, but this change breaks package public interface, which can be confusing for the current consumers.

@CraigRichards

Copy link
Copy Markdown
Contributor Author

Hi @CraigRichards,
Idea is good, but this change breaks package public interface, which can be confusing for the current consumers.

Hi,
Can you please check the commit again? I left the old method in place and added an overload?
This will not break any existing consumers.

If you review you the code, I left the existing test in place 'AllureLifeCycleTest' which calls the original method. So your existing contract remains valid.

Then I duplicated this test and called it 'AllureLifeCycleTestUsingNewAddScreenDiff' which tests the new method.

Thanks and looks forward to contributing further.

Cheers
Craig

@delatrie

Copy link
Copy Markdown
Contributor

That overload has been introduced as part of #371. Released in 2.10.0

@delatrie delatrie closed this Dec 27, 2023
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.

4 participants