-
Notifications
You must be signed in to change notification settings - Fork 233
refactor: reimplement csv option for domains #3388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note: The sorting for CSV is different from the table sorting, so the outputs don't match exactly. The table is sorted by oclif/table, which uses the |
zwhitfield3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work here! Well organized and refactored. I requested a few changes, but otherwise, this PR is looking good! I think we should maintain the same sorting between the table and CSV to maintain consistency.
|
@zwhitfield3 I updated the code with your suggestions and added |
zwhitfield3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you for making those updates!
Description
W-19830244
This PR reimplements
--csvforheroku domains, since it was removed from oclif during the migration to v4. It allows users to output domain data in CSV format and export to a.csvfile.Type of change:
Refactor
Testing
NOTE: You will need an app with multiple domains (at least 3-4 to test sorting).
Test CSV output:
npm install && npm run build./bin/run domains -a <your-app> --csv, confirm CSV output with headers./bin/run domains -a <your-app> --csv > test-output.csv, confirm file was created successfully and opens with proper CSV formatTest CSV sorting:
./bin/run domains -a <your-app> --csv --sort "Domain Name", confirm CSV output was sorted by the Domain Name column (direct property)./bin/run domains -a <your-app> --csv --sort "DNS Record Type", confirm CSV output was sorted by the DNS Record Type column (not a direct property)Test CSV with other flags:
./bin/run domains -a <your-app> --csv --extended, confirm output includes additional columns (ACM Status, ACM Status Reason)./bin/run domains -a <your-app> --csv --columns "Domain Name,DNS Target", confirm output only shows specified columns./bin/run domains -a <your-app> --csv --filter "Domain Name=example", confirm output only shows domains matching the filter criteriaVerify no regressions:
./bin/run domains -a <your-app>, confirm table output still works./bin/run domains -a <your-app> --sort "Domain Name", confirm table sorting still works