Separate determining FQDN, add tests, make FQDN regex optional

etsy / opsweekly

On call alert classification and reporting

MIT License

761 stars 100 forks source link

Separate determining FQDN, add tests, make FQDN regex optional #48

Closed KevinMGranger closed 4 years ago

KevinMGranger commented 9 years ago

This relates to #47.

One very notably change is the introduction of Autoloading and Namespacing. Those are tenets of modern, best-practices PHP, but does entail running composer install before the project is usable.

I don't think that's too much to ask, considering there's already a composer.phar included in the repo (although I'd prefer everyone had their own copy of composer).

Thoughts?

mrtazz commented 9 years ago

Thank you for the contribution! I left a couple of comments on the code, but overall this looks great.

KevinMGranger commented 9 years ago

Changes made!

Thoughts on ./composer.phar in the README, versus telling people to install composer on their own? (And removing composer.phar) ?

mrtazz commented 9 years ago

the changes look good to me now. I think it makes sense to have everyone install their own composer and not track this in here. But I don't feel super strongly about this. We can discuss it in a separate pull request to not block this one with this question. @lozzd did you have any additional comments on this change?