search

CrazyAwesomeCompany / autophing

Automatic Phing targets
13 stars 4 forks source link

Feature/checkreturn #9

Closed nstapelbroek closed 9 years ago

nstapelbroek commented 9 years ago

Hey Nick,

We've recently switched to Travis for our Continuous Integration needs. As you might know, Travis checks the return value of the executed build scripts to determine if the build has succeeded or failed. So I've taken the liberty to introduce a checkreturn in each exec statement of the autophing libraries.

In addition, the passthru and checkreturn properties can be configured on a library basis (instead of a global passthru).

Things you should know:

– I've only tested this in my local phpunit / phpcs setup on a recent project. So I'm not sure if this actually works for tools like phplint or swagger. – Merging this WILL BREAK backward compatibility. So ensure to bump the major version when releasing a merged PR. – $library$.passthru has now a default value of true to ensure maximum backward compatibility – $library$.checkreturn has now a default value of false to ensure maximum backward compatibility

p.s. @eXistenZNL could you give your opinion / review on this PR?

eXistenZNL commented 9 years ago

Nick, dit is hoe Nico en ik het besproken hebben. Nice.

De enige reden waarom dit breekt is als iemand expliciet in z'n build.xml passtru op false gezet had, die property zou nu niet meer gehonoreerd worden omdat dit nu is vervangen voor een 'genamespacete' passtru en checkreturn per autophing-module.

Passtru was tot nu toe nergens gedefined wat het geheel af liet hangen van wat magie, en defaulte per ongedocumenteerde Phing-regels naar true, bleek naar wat uitzoekwerk. Om deze onoverzichtelijkheid / edge case weg te nemen, staat het nu expliciet overal in.

Om dit hele zaakje backwards compatible te krijgen met hoe het nu is was de mogelijkheid om in elke .properties-file de globale variabelen checkreturn en passtru te introduceren, met hun default state die overeen zou komen met hoe het nu werkt. Echter overschrijf je dan met de ene properties file weer de andere, en is het nog niet echt fraai.

Nu is per module de properties file los 'genamespacet' dus overschrijf je nooit onoverzichtelijk een property uit een andere file en kun je kiezen welke mogen breken en welke niet.

Door bovenstaande backwards compatibility break kom je uit op versie 2.0.0, zoals beschreven is op http://semver.org/. Ook Composer wordt daar blij van: https://getcomposer.org/doc/articles/versions.md#caret En als je em merget maakt dat Nico weer blij. En het maakt jou weer blij, want gratis code. Mij maak je niet blij, want je kauwgomballenautomaat is leeg. Kom die eens bijvullen!

eXistenZNL commented 9 years ago

Psssst @ngroot!

ngroot commented 9 years ago

Hi Nico,

This is great! Thank you!

eXistenZNL commented 8 years ago

Thanks for merging gek!

eXistenZNL commented 8 years ago

@ngroot Tag je em ook nog effe zodat we 'm als dist binnen kunnen harken van packagist? Nu zijn we verbannen tot dev-master, wat een git clone veroorzaakt en alles traag maakt :)

Thx in advance!

eXistenZNL commented 8 years ago

Oeps deze monkey heeft weer eens niet goed gekeken, je had al netjes versie 2.0 uitgebracht. Sorry!