-
Notifications
You must be signed in to change notification settings - Fork 700
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
Fix #8928 Conclude setup messages with an ellipsis #8929
Conversation
Thank you! I'm seeing a good bunch of reference tests results holding the |
7dad98c
to
28ed9fa
Compare
@ulysses4ever, I took the search and replace route. After a couple of false starts, the checks now pass. |
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.
Terrific!
@mpilgrem Hi, do you need help with rebasing this PR? I can offer my help. |
@Kleidukos, I had not spotted that. I'll look at it over the coming weekend and let you know. |
@mpilgrem thanks a lot for updating the PR and sorry it took so long to get all the approvals! There is a couple tests that have been added in the meantime and they have to be fixed, see the CI but I think the relevant bit is:
|
Also adds ellipsis to lines in *.out files.
@mpilgrem thanks a lot for attending to it! One thing for the future: normally, we don't manually merge PRs but rather employ a merge bot. So, when you get 2 approvals and a green CI, you're expected to put one of the two labels: |
@ulysses4ever, sorry - I did not appreciate that (about merging). EDIT: I should have read the |
Bonus points for added automated tests!