Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CSF Tools: Use recast for printing in csf-tools #23427

Merged
merged 5 commits into from Jul 28, 2023
Merged

CSF Tools: Use recast for printing in csf-tools #23427

merged 5 commits into from Jul 28, 2023

Conversation

kasperpeulen
Copy link
Contributor

@kasperpeulen kasperpeulen commented Jul 12, 2023 •
edited

What I did

How to test

See the changed snapshots since we use the new method printCsf and printConfig.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@kasperpeulen kasperpeulen added build Internal-facing build tooling & test updates ci:normal labels Jul 24, 2023
JReinhold
JReinhold reviewed Jul 24, 2023
View reviewed changes
Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like it would be good to have tests for printCsf and printConfig? 🚓

@kasperpeulen
Copy link
Contributor Author

@JReinhold Made sure that printConfig and printCsf are now both used in a lot of tests.

@vanessayuenn
Copy link
Contributor

@kasperpeulen can you fill out the "how to test" section please? If it's all covered by unit tests, you can write it as such. Thanks!

@kasperpeulen kasperpeulen added ci:merged Run the CI jobs that normally run when merged. and removed ci:normal labels Jul 27, 2023
@kasperpeulen kasperpeulen merged commit d8d8cac into next Jul 28, 2023
71 checks passed
@kasperpeulen kasperpeulen deleted the kasper/recast-refactor branch July 28, 2023 08:45
@github-actions github-actions bot mentioned this pull request Jul 28, 2023
Merged
16 tasks
@vanessayuenn vanessayuenn added maintenance User-facing maintenance tasks and removed build Internal-facing build tooling & test updates labels Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shilman shilman shilman left review comments

@JReinhold JReinhold JReinhold left review comments

@ShaunEvening ShaunEvening ShaunEvening left review comments

@ndelangen ndelangen Awaiting requested review from ndelangen

@tmeasday tmeasday Awaiting requested review from tmeasday

Assignees

@kasperpeulen kasperpeulen

Labels
ci:merged Run the CI jobs that normally run when merged. maintenance User-facing maintenance tasks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@kasperpeulen @vanessayuenn @shilman @JReinhold @ShaunEvening