Port Bash scripts to PHP#1591
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1591 +/- ##
=========================================
Coverage 97.43% 97.44%
- Complexity 986 988 +2
=========================================
Files 213 213
Lines 2265 2273 +8
=========================================
+ Hits 2207 2215 +8
Misses 58 58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This code is awful, but I'm not willing to prioritize refactoring it, since it's not some critical part of the library. |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the development tooling by converting Bash scripts to PHP Symfony Console commands and refactors the Tld and PostalCode validators to load data from external files rather than hardcoded arrays. This change makes the codebase more maintainable and easier to update, as data can be regenerated without modifying class files.
Key Changes:
- Replaced Bash scripts in
bin/with Symfony Console commands indev/Commands/ - Refactored
TldandPostalCodevalidators to load data from PHP files indata/domain/ - Added new
bin/consoleentry point for all development commands - Updated GitHub workflow to use new console commands
Reviewed changes
Copilot reviewed 14 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| bin/console | New entry point for Symfony Console application with all development commands |
| dev/Commands/*.php | New Console commands for updating TLDs, postal codes, domain suffixes, doc links, and generating mixins |
| library/Validators/Tld.php | Refactored to load TLD list from external data file instead of hardcoded array |
| library/Validators/PostalCode.php | Refactored to load postal codes from external data file |
| data/domain/tld.php | New data file containing TLD list |
| data/domain/postal-code.php | New data file containing postal code patterns |
| composer.json | Added symfony/console dependency and dev autoload configuration |
| phpcs.xml.dist | Added dev/ directory to code standards checks |
| .github/workflows/update-regionals.yaml | Updated to use new console commands |
| .gitattributes | Restructured to use allow-list approach for exports |
| docs/09-list-of-validators-by-category.md | Reorganized validator categories |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
d0325c5 to
25d5ab8
Compare
With that change, we won't need to change the Tld validator every time there's an update.
25d5ab8 to
f82eeda
Compare
ec992a9 to
0a84b14
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 72 out of 78 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
With this change, we won't need to change the `PostalCode` validator every time there's an update. While I was moving the data, I noticed some inefficiencies with the regular expressions, so I made some changes.
It makes more sense to use PHP to generate PHP code than to use Bash. I love writing Bash scripts, but I know it's not for everyone, and they can become quite complex. Porting them to PHP code also lowers the barrier for people to change them. While I was making those changes, I also noticed a problem with how we save the domain suffixes. We're converting all of them to ASCII, so we are not preserving languages such as Chinese, Thai, and Hebrew, which use non-ASCII characters.
0a84b14 to
13f2fa6
Compare
|
Looks great!
Can we add a test that ensures validation for those new languages work? Just a few for the most stable international domains. |
I manually run the commands that update the data we use in `Tld`, `PostalCode`, and `PublicDomainSuffix`.
13f2fa6 to
f635cc7
Compare
It makes more sense to use PHP to generate PHP code than to use Bash. I love writing Bash scripts, but I know it’s not for everyone, and they can become quite complex. Porting them to PHP code also lowers the barrier for people to change them.
While I was making those changes, I also noticed a problem with how we save the domain suffixes. We’re converting all of them to ASCII, so we’re not preserving languages such as Chinese, Thai, and Hebrew, which use non-ASCII characters.