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

[Sylius/Behat] Use getUrl() instead of custom open() in ProductShowPage #4090

Merged
merged 1 commit into from
Feb 10, 2016

Conversation

pamil
Copy link
Contributor

@pamil pamil commented Feb 9, 2016

No description provided.

@pjedrzejewski pjedrzejewski added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Feb 9, 2016
{
return $this->router->generate($urlParameters['product'], [], true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add check if product is set?

if (!isset($urlParameters['product']) {
  \\ throw exception
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@pamil pamil force-pushed the product-show-page-get-url branch 3 times, most recently from d4b47c7 to 8e07dd0 Compare February 10, 2016 12:08
@pamil
Copy link
Contributor Author

pamil commented Feb 10, 2016

Something went wrong with Travis, here's the passing build on my fork: https://travis-ci.org/pamil/Sylius/jobs/108260994

@michalmarcinkowski
Copy link
Contributor

@pamil the passing build from your fork is not enough 😄 please repush, should be fine then.

michalmarcinkowski added a commit that referenced this pull request Feb 10, 2016
[Sylius/Behat] Use getUrl() instead of custom open() in ProductShowPage
@michalmarcinkowski michalmarcinkowski merged commit 6ec0211 into Sylius:master Feb 10, 2016
@michalmarcinkowski
Copy link
Contributor

👍

@pamil pamil deleted the product-show-page-get-url branch March 18, 2016 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants