Improved email validation #156
Conversation
mredig
force-pushed
the
master
branch
2 times, most recently
from
728ef75
to
1aac95e
Compare
There was a problem hiding this comment.
Thanks for these additions! A few comments before going deeper:
- could you use the
///documentation syntax instead of/***/to follow the project convention? - there's a few SwiftLint warnings, could you fixed them? If you don't want to have it on your Mac you can check the warnings in the file changes
- could you add the watchOS test target or remove the testing run in the CI?
I will finish the review once that's done, the warnings in the file changes make the string extensions changes harder to read
|
In the process of adding watchOS testing :) I'll get the other requests done soon as well. |
|
One more request: could you also update the README and CHANGELOG? 🙏 |
|
Just so you know, the version of SwiftLint that was in here before was outdated. (looks like it was last updated sometime around 2020, maybe?) This new version has created a few warnings, but everything showing in GitHub is no longer in any of the contributions I made. I'm pretty sure these actions should pass, so it should be good to go now. |
|
@mredig I have one last request (last one, promise): could you squash your commits in a few ones instead of the current 45 to keep a clean history? Beside that, it's all good, ready to go! |
(refactor/feat) improved email detection and added domain detection (refactor) adjust cyclical complexity due to swiftlint warning (refactor) mark availability of test (refactor) updated xcode to match the file adjustments made from within spm (refactor) swiftlint works when building in xcodeproj might have been an m1 issue, but the build script didn't know the path for swiftlint and failed the build (fix) restrict domain validation to correct versions (fix) CI env settings (refactor) run SPM tests in ci (nit) line length (fix) platform requirements for ci tests (refactor) specifc macos runner os (refactor) simplify the iphone test (refactor) xcode select before doing anything and tweak other test commands (fix) quotes (fix) key/value format (refactor) tweak xcode path (refactor/test) try switch macos and spm targets (fix) remove space in platform names (refactor/debug) print current xcode select before changing (feat) created watch os tests (refactor) support tests in watchos scheme (nit) documentation comment style (nit) address swiftlint function length warning (nit) address swiftlint switch indentation warning (nit) swiftlint line length (nit) remaining swiftlint warnings disabled them because the algorithm is necessarily complex, so the complexity issue is unfortunate but necessary. The line length is a side effect of that (I COULD put the parseemailsections method outside the current placement of being embedded, but it's only ever used for email validation, so I think this is better code org) (nit) specify file as space indentation (nit) indentation (nit) bonus swiftlint warning (refactor) add to changelog and readme (fix) remove file from watchos tests with incompatibilities (refactor) removed unnecessary sequence requirement on cicd (refactor) address swiftlint linelength issues AND fix deprecated minimum versions (refactor) run a current version of swiftlint in cicd (fix) include watchos files (fix) add brew to path in swiftlint runner (fix) remove watchos incompatible files (fix) watchos incompatibility (refactor) output swiftlint to github ... hopefully! (nit) swiftlint warnings (fix) watchos test build (nit) indentation (nit) swiftlint line length (nit) indentation
Sorry - didn't see this until now. Personally, I'm not a fan of modifying the history since it completely obviates the entire purpose of git, but it's your project, not mine! |
Added simple domain validation as a supporting feature, and made testing compatible with a macOS build target.