techvalidate / pano

MIT License
0 stars 0 forks source link

Email validator updates #20

Closed mkmiller closed 7 years ago

mkmiller commented 7 years ago

This PR disallows emails with double periods and allows TLDs longer than 4 characters.

I plan to add tests to CX.

@jmckible Do you want to do reviews on things like this in the future?

mkmiller commented 7 years ago

I don't mind adding tests to pano, though would we want to convert over to rspec?

Tests would then run on a model in the dummy, right?

jmckible commented 7 years ago

Yeah, rspec for consistency/sanity is advisable.

I believe model tests can go right in the root dir. I think only function/request specs need to load up the dummy app http://guides.rubyonrails.org/engines.html#testing-an-engine

mkmiller commented 7 years ago

@jmckible RSpec is now supported: you can rake spec in root. I also moved the dummy app into spec/, which has the unfortunate side effect of breaking pow. Worth it? Also, is travis script in scope?

jmckible commented 7 years ago

Looks good, @mkmiller. Sending back to you for README update and socializing the pow symlink change.