Skip to content

Update namespace for Allure adapter - #42

Merged
baev merged 2 commits into
allure-framework:masterfrom
okolesnyk:master
Dec 18, 2018
Merged

Update namespace for Allure adapter#42
baev merged 2 commits into
allure-framework:masterfrom
okolesnyk:master

Conversation

@okolesnyk

Copy link
Copy Markdown
Contributor

By adding PHPUnit to namespace we resolving issue: #41

 - PHPUnit allure adapter conflicting with Codeception allure adapter
@okorshenko

Copy link
Copy Markdown

Thank you for fixing the issue. Looking forward for this solution to be merged! 👍

<?php

namespace Yandex\Allure\Adapter;
namespace Yandex\Allure\PHPUnit\Adapter;

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.

I would like to have Yandex\Allure\PHPUnit namespace instead as well as class named AllurePHPUnit (or PHPUnit replaced with PhpUnit in both cases if it is applicable in PHP world)

<?php

namespace Yandex\Allure\Adapter;
namespace Yandex\Allure\PHPUnit\Adapter;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
namespace Yandex\Allure\PHPUnit\Adapter;
namespace Yandex\Allure\PHPUnit;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@baev does it look better now? (can you see suggested code?)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I like PhpUnit more than PHPUnit :)

@baev baev Dec 17, 2018

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.

👍

PS Any thoughts on replacing PHPUnit with PhpUnit? to late 😄

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@okolesnyk will update the code shortly
@baev thank you for review

 - PHPUnit allure adapter conflicting with Codeception allure adapter
 - Code review changes
@okolesnyk

Copy link
Copy Markdown
Contributor Author

Hi @baev and @okorshenko
I did change PHPUnit to PhpUnit and renamed class to AllurePhpUnit.
Please take a look and let me know if it looks good or not.
Thanks

@okorshenko

Copy link
Copy Markdown

Hi @okolesnyk Thank you for quick fixes
Looks good from my side

Waiting for @baev feedback

@baev
baev merged commit 8c59b72 into allure-framework:master Dec 18, 2018
@baev

baev commented Dec 18, 2018

Copy link
Copy Markdown
Member

@okolesnyk @okorshenko thanks!

@baev

baev commented Dec 18, 2018

Copy link
Copy Markdown
Member

if there any other updates needed (like dependency updates) feel free to submit PR. Also it would be really appreciated if you guys review #38

I'll release the changes tomorrow

@okorshenko

Copy link
Copy Markdown

Hi @baev
Looking forward for new release!
I will review #38 today. From the glance view - looks reasonable, but I want to double-check rules on versioning

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