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.

Sign up for GitHub

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

Jump to bottom

[PHP] Check skipping artifiactVersion in other mustache templates, since it is not required anymore #2947

Merged
merged 12 commits into from
Jun 1, 2016
Merged

[PHP] Check skipping artifiactVersion in other mustache templates, since it is not required anymore #2947

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cce21ab
Removed printing artifactVersion if it is not set
May 24, 2016
c54b4fa
Removed printing artifactVersion if it is not set from debug information
May 24, 2016
18f32b4
Proper printing of invokerPackage in configuration debug report
May 24, 2016
3484704
Updated condition for artifactVersion check
May 24, 2016
030b1bc
added default version 1.0.0 to userAgent
May 26, 2016
8369bae
Improved formatting
May 30, 2016
794e7e4
Merge pull request #3 from swagger-api/master
May 30, 2016
ce2e994
Removed printing artifactVersion if it is not set
May 24, 2016
37264b0
Removed printing artifactVersion if it is not set from debug information
May 24, 2016
4f54e5c
Improved formatting
May 30, 2016
9f94220
Merge branch 'version-check-mustache' of github.com:mikolajprzybysz/s…
Jun 1, 2016
4757647
Replaced double quotes with singles
Jun 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/swagger-codegen/src/main/resources/php/README.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
This PHP package is automatically generated by the [Swagger Codegen](https://github.com/swagger-api/swagger-codegen) project:

- API version: {{appVersion}}
- Package version: {{artifactVersion}}
- Build date: {{generatedDate}}
{{#artifactVersion}}- Package version: {{artifactVersion}}
{{/artifactVersion}}- Build date: {{generatedDate}}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just put {{#artifactVersion}} and {{/artifactVersion}} on lines of their own.

As of recently whitespace will be stripped around them if they are the only thing on a line.

This looks nicer and more readable in the mustache template.

- Build package: {{generatorClass}}
{{#infoUrl}}
For more information, please visit [{{{infoUrl}}}]({{{infoUrl}}})
Expand Down
8 changes: 4 additions & 4 deletions modules/swagger-codegen/src/main/resources/php/configuration.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class Configuration
*
* @var string
*/
protected $userAgent = "{{#httpUserAgent}}{{{.}}}{{/httpUserAgent}}{{^httpUserAgent}}Swagger-Codegen/{{{artifactVersion}}}/php{{/httpUserAgent}}";
protected $userAgent = "{{#httpUserAgent}}{{{.}}}{{/httpUserAgent}}{{^httpUserAgent}}Swagger-Codegen{{#artifactVersion}}/{{{.}}}{{/artifactVersion}}/php{{/httpUserAgent}}";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should put in something instead of the artifactVersion when it is not set.

I think I read something about the default user agent is intended to follow a format of Swagger-Codegen/version/language (but I can't find the reference for that now).

So maybe we should just put in the string "unknown-version" when we don't have a better guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep the default version (1.0.0) to make it consistent with other langauges and have the users set the version properly via artifactVersion


/**
* Debug switch (default set to false)
Expand Down Expand Up @@ -513,11 +513,11 @@ class Configuration
*/
public static function toDebugReport()
{
$report = "PHP SDK ({{invokerPackage}}) Debug Report:\n";
$report = 'PHP SDK ({{invokerPackage}}) Debug Report:'."\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

What about replacing "\n" with PHP_EOL? (I thought you've done it via another PR already, maybe you need to rebase on the latest master)

What about remaining lines with double quote? shall we replace it with single quote as well?

$report .= " OS: ".php_uname()."\n";
$report .= " PHP Version: ".phpversion()."\n";
$report .= " OpenAPI Spec Version: {{version}}\n";
$report .= " SDK Package Version: {{artifactVersion}}\n";
$report .= " OpenAPI Spec Version: {{version}}\n";{{#artifactVersion}}
$report .= " SDK Package Version: {{artifactVersion}}\n";{{/artifactVersion}}
$report .= " Temp Folder Path: ".self::getDefaultConfiguration()->getTempFolderPath()."\n";

return $report;
Expand Down