-
Notifications
You must be signed in to change notification settings - Fork 1
Use JsonFileHelper to read/write manifests #50
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
Conversation
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.
Za me ok, jen mozna jinde v testech to muze zpusobit problem, ze ten manifest je PRETTY_PRINT
.
To je asi pravda. Dobrý catch, díky 👍 |
50aedc9
to
eb101b9
Compare
Upravil jsem na nepretty print, nechám projít testy a mergnu. |
tak to nechapu proc je state file pretty print a manifest ne |
Protože state file je new feature. Formátovaný manifest by byl BC break. |
proc by to byl bc break? |
Protože tam už předtím dumpování manifestu bylo a formátované nebylo. |
a co? funkcne je to stejny, runneru je to jedno, neni duvod aby na to nekde neco spolihalo a i kdyby, tak se na to prijde pri updatu te konretni komponenty. prece nebudeme uz na vecnost generovat jeden json pretty a druhy compressed |
Ja jsem za ten pretty one. Hlavne kvuli tomu, ze kdyz debuguju testy, tak se v tom muzu dobre orientovat, oproti 1000 znaku dlouhemu radku. Jak pise @odinuv, kde to je otestovane, tak na to prijdeme pri updatu konkretnich komponent. Kde na to testy nejsou, tak si toho ani nevsimneme. Tak jak tak muzeme byt v klidu, protoze to nic rozbit nemuze (doufam, ze nemame nejaky custom parser na json, ktery podporuje jen one liner 😃 ) I ten state.json puvodne byl one liner a ted jsme to prehoupli na pretty print. |
A je to :) |
Fixes #49