Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added convenience overload for AddScreenDiff #231

Conversation

CraigRichards
Copy link
Contributor

@CraigRichards CraigRichards commented Jun 9, 2021

//: # (
. 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
Copy link

CLAassistant commented Jun 9, 2021

CLA assistant check
All committers have signed the CLA.

@Bakanych
Copy link
Contributor

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

@CraigRichards
Copy link
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
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