-
Notifications
You must be signed in to change notification settings - Fork 7
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
Initial repository setup #1
Conversation
c3c8ee8
to
1b7fe74
Compare
- travis_retry composer update --no-interaction $DEPENDENCIES | ||
|
||
script: | ||
- vendor/bin/phpunit --coverage-clover coverage-clover.xml |
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.
osobne mam radsi, kdyz je tohle nejak zabalene pod composer test
napr
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.
Jo, jenze tohle je specificky nastaveni pro travis (coverage se jinak negeneruje).
A zabalovat vendor/bin/phpunit do "composer test" mi neprijde moc ucelny :)
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.
no ja v ramci toho testu beru i code style check a dalsi veci.. zkratka celou "kompilaci"
- navic mi prijde mozna zbytecne nemit ten code-coverage primo v configu pro phpunit
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.
No to prave typicky nemusis chtit, protoze coverage se vetsinou generuje jen na CI, protoze to jinak zdrzuje (a musis mit lokalne v CLI xdebug, coz nechces).
To se da resit pres phpunit.xml.dist a lokalni phpunit.xml , ale prijde mi lepsi mit to takhle obracene - by default vypnuty, na CI explicitne zapnuty.
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.
no ja zase prave ve stormu kdyz poustim testy, tu coverage chci, aby mi obarvil kod, ktery prosel
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.
pak ti ale nic nebrani udelat si composer cli
, kde budes mit tyhle casy poresene a mas nad tim kontrolu
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.
Jasne, ale mne jde o minimalni default, aby kdyz pustim vendor/bin/phpunit, tak to spustilo testy, a nedelalo zadny dalsi kejkle, nepsalo chyby ze chybi xdebug atd.
Pokud potrebuje nekdo lokalne neco specifickyho, necht si udela lokalni phpunit.xml nebo to spusti se specifickym CLI prikazem. Ale tezko najit kompromis, kdyz to kazdy pouziva jinak, nez "minimalni varianta", ktera tu je.
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.
jo to asi klidne, na druhou stranu, kdyz tady zname presne dany use-case, proc ho nedefinovat konkretne do composer cli
prikazu
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.
Toto je ale specificke chovani prave a jenom tohoto buildu pro Travis, nikoliv obecne pro CLI ani pro nic jineho - tudiz .travis.yml mi prijde jako to spravne misto na to, kde to mit definovane :).
@@ -0,0 +1,42 @@ | |||
{ | |||
"name": "lmc/matej", |
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.
neni to zbytecne konkretni? co kdyz budou dalsi knihovny tykajici se mateje? nebylo by lepsi napr lmc/matej-api-client
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.
Jako jake napriklad knihovny? Zase mi prijde zbytecny delat to specificky, kdyz zadna takova knihovna nejspis nikdy nebude :). A bundle nazveme jinak.
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.
jo muze byt.. jen jsem na to chtel upozornit, jestli nekdo treba nevi o necem uz ted.. protoze tohle je neco co se v budoucnu dost blbe meni
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.
a tim, ze tomu das konkretnejsi nazev taky zarucis i SRP
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.
Uvidime, promyslel jsem to, ale zase jsem spise pro jednoduchost, nez tomu ted davat nejaky premature specificky nazev :).
}, | ||
"require-dev": { | ||
"php-http/guzzle6-adapter": "^1.1", | ||
"phpunit/phpunit": "^5.7 || ^6.4", |
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.
proc i 5.7
?
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.
Protoze podporuje PHP 5.6.
Viz vyse - pak musim(e) vymyslet, jak bude udelana ta tanspilace.
"Lmc\\Matej\\": "tests/" | ||
} | ||
}, | ||
"require": { |
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.
co PHP verze?
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.
Tohle poresim, az budeme resit tu transpilaci pro 5.6 - ted jeste nevim, jak to bude.
"phpstan/phpstan-shim": "^0.8.5", | ||
"php-coveralls/php-coveralls": "^1.0" | ||
}, | ||
"scripts": { |
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.
pridal bych i "test": ["@check", "@phpunit"]
a "phpunit": "..."
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.
A tak ok, proc ne :).
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.
Je to tam, ale ten phpunit mi prijde porad zbytecnej do toho balit :D
@@ -0,0 +1,18 @@ | |||
<phpunit |
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.
co pridat i
convertErrorsToExceptions = "true"
convertNoticesToExceptions = "true"
convertWarningsToExceptions = "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.
phpunit.xml.dist
Outdated
<directory suffix=".php">./src</directory> | ||
</whitelist> | ||
</filter> | ||
</phpunit> |
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.
jeste pouzivam
<php>
<!-- E_ALL = 30719 -->
<ini name="error_reporting" value="30719"/>
</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.
Je to tam.
src/Matej.php
Outdated
/** @var string */ | ||
private $apiKey; | ||
|
||
/** |
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.
v PHP 7 bych se anotacim vyhnul, pokud mas typovou kontrolu primo v metode, je to redundantni informace
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.
Jo, urcite, dam pryc. A teda doufame, ze pri ty transpilaci se tam zase budou generovat :)?
80a5ae2
to
ba3fc5b
Compare
No description provided.