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

[WIP] Varnish Vcl generator command #9286

Conversation

piotrkwiecinski
Copy link
Contributor

@piotrkwiecinski piotrkwiecinski commented Apr 17, 2017

Add new CLI command to allow generation of Varnish VCL configuration

Description

In addition to new CLI to generate VCL file same generator is now used in admin UI.

Fixed Issues (if relevant)

  1. Create New CLI command: Generate Varnish VCL file #9199: Create New CLI command: Generate Varnish VCL file

Manual testing scenarios

  1. bin/magento varnish:vcl:generate

Automated tests will be added tomorrow

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

self::BACKEND_PORT_OPTION,
null,
InputOption::VALUE_REQUIRED,
'Backend post for configuration',
Copy link
Contributor

@adragus-inviqa adragus-inviqa Apr 18, 2017

Choose a reason for hiding this comment

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

Typo: port, not post. Also, I had a bit of problem understanding the phrase Backend port for configuration. Can we be more explicit? I know what it does, but I just think some people will need more info about it. Don't be terse on documentation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the comment. Do you have suggestions how to better call it?

@vrann vrann self-assigned this Apr 18, 2017
@vrann vrann added this to the April 2017 milestone Apr 18, 2017
@vrann vrann added the develop label Apr 18, 2017
@magento-team magento-team merged commit b95d230 into magento:develop Apr 19, 2017
magento-team pushed a commit that referenced this pull request Apr 19, 2017
- fixed static and integration tests
- moved interfaces to proper namespace
magento-team pushed a commit that referenced this pull request Apr 19, 2017
- fixed static and integration tests
- moved interfaces to proper namespace
magento-team pushed a commit that referenced this pull request Apr 19, 2017
- fixed static and integration tests
magento-team pushed a commit that referenced this pull request Apr 19, 2017
- fixed static and integration tests
magento-team pushed a commit that referenced this pull request Apr 19, 2017
- fixed static and integration tests
@magento-team
Copy link
Contributor

@piotrkwiecinski thank you for your contribution

@miguelbalparda
Copy link
Contributor

@piotrkwiecinski I just saw this and I wanted to understand the code better. I couldn't find a switch for the admin value in the code and I wanted to know if this is not necessary or it is just not taken into account in this PR. Thanks!

@Ctucker9233
Copy link

@magento-team I see this PR isn't in 2.2 as of 2.2.7. Is there a backport or is this a 2.3 fix only?

@piotrkwiecinski piotrkwiecinski deleted the generate-varnish-vcl-console-command branch March 21, 2019 16:14
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.

6 participants