refactored a little, added CI actions, dummy test #51
Conversation
.github/workflows/phpunit-sqlite.yml
Outdated
| php-versions: ['8.2'] | ||
| server-versions: ['master'] |
There was a problem hiding this comment.
You could test against all supported Php and NC:
Php >= 8.0
NC >= 26
There was a problem hiding this comment.
php-versions: ['8.1', '8.2']
server-versions: ['stable26', 'stable27', 'master']
I think will be enough...
| public function __construct(ITimeFactory $time, | ||
| JiraAPIService $jiraAPIService, | ||
| LoggerInterface $logger) { | ||
| JiraAPIService $jiraAPIService, | ||
| LoggerInterface $logger) { |
There was a problem hiding this comment.
Since you know Php >= 8.0 will be used you could use attribute declaration in construct params. Not very important though.
There was a problem hiding this comment.
I fixed missing type declarations in all files, removing them completely.
Declaring in construct params - let leave this for the next time.
| IRequest $request, | ||
| IConfig $config, | ||
| IURLGenerator $urlGenerator, | ||
| IL10N $l, | ||
| NetworkService $networkService, | ||
| ?string $userId) { |
There was a problem hiding this comment.
| IRequest $request, | |
| IConfig $config, | |
| IURLGenerator $urlGenerator, | |
| IL10N $l, | |
| NetworkService $networkService, | |
| ?string $userId) { | |
| IRequest $request, | |
| private IConfig $config, | |
| private IURLGenerator $urlGenerator, | |
| private IL10N $l, | |
| private NetworkService $networkService, | |
| private ?string $userId) { |
(Just in case) In Php >= 8.0, if you do this you can get rid of the property declaration (before the constructor) and of the property assignment (in the constructor).
There was a problem hiding this comment.
Yeah, I know but I like the old style for some reason more..
Most stuff was taken from picker
Test was added only one, as it is more likely a test for a test.
Linters and style checking stuff is much more useful :)
P.S: I know that commits should be separated, but this should be easy review I hope even in this bad case.