-
Notifications
You must be signed in to change notification settings - Fork 2
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
#93411 Cleanup, php cs fixer, pest #6
Conversation
@@ -4,7 +4,6 @@ composer.lock | |||
npm-debug.log | |||
yarn-error.log | |||
.php_cs.cache | |||
composer.lock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а зачем это из игнора убрал?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
он там 2 раза указывался, на 3 строке есть
|
||
return (new PhpCsFixer\Config()) | ||
->setRules([ | ||
'@PSR2' => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
скопируй конфиг из наших сервисов https://gitlab.com/greensight/ensi/units/admin-auth/-/blob/master/.php-cs-fixer.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сделал, до этого был от серверного генератора, надо бы его там тоже будет обновить
composer.json
Outdated
"scripts": { | ||
"test": "phpunit" | ||
} | ||
"name": "ensi/laravel-openapi-client-generator", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
тут надо вернуть пробелы, сейчас появились табы откуда-то
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
там до этого было в разнобой вообще, сделал 4 пробела вместо табов
$patcher = new NodeJSEnumPatcher($file); | ||
$patcher->patch(); | ||
} catch (Exception) { | ||
$this->info("Patch enum: $file\t[SKIP]"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Выглядит так, как будто тут должен быть "error" и вывод текста ошибки ещё
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
добавил вывод ошибки
src/Commands/GeneratePhpClient.php
Outdated
|
||
$this->disableComposerPatchRequire = (bool) config('openapi-client-generator.php_args.composer_disable_patch_require', false); | ||
$this->composerName = config("openapi-client-generator.{$this->client}_args.composer_name", 'ensi/openapi-client-php-example'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
мне кажется что тут лучше не невалидное значение по-умолчанию делать, а кидать исключение, если не задано значение в конфиге
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
убрал значение по-умолчанию, теперь будет ошибка если этот параметр не задан
src/Commands/GeneratePhpClient.php
Outdated
$patcher = new PhpEnumPatcher($file); | ||
$patcher->patch(); | ||
} catch (Exception) { | ||
$this->info("Patch enum: $file\t[SKIP]"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
аналогично
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
добавил вывод ошибки
} | ||
|
||
file_put_contents($this->enumFile, $enum); | ||
} | ||
|
||
private function patchConstantProperties(string $enum, string $value, string $name, string $title): string | ||
{ | ||
$enum = preg_replace( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не поняла что в итоге делает этот класс, если это удалено
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Решили в чате пока патчеры енамов оставить как есть
|
||
/** | ||
* @var string | ||
*/ | ||
protected $manifestName = 'package.json'; | ||
|
||
/** | ||
* @var boolean | ||
* @var bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в этом файле тоже можно вынести типизацию на уровень самого php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сделал
tests/TestCase.php
Outdated
|
||
public function getEnvironmentSetUp($app): void | ||
{ | ||
config()->set('openapi-client-generator.apidoc_dir', ('./tests/api-docs')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
лишние скобочки круглые
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
поправил
tests/TestCase.php
Outdated
// PHP client params | ||
config()->set('openapi-client-generator.php_args.params', [ | ||
'apiPackage' => 'Api', | ||
'invokerPackage' => 'Baristanko\\OpenapiClientPHPExample', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Давай без Baristanko, пусть тут будет Ensi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
изменил
Обновление не ломает обратную совместимость, по-сути тут просто причесывание старого кода