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

Revert "Fix #38256 (lpad/rpad defined in terms of textwidth)" #41131

Closed
wants to merge 1 commit into from
Closed

Revert "Fix #38256 (lpad/rpad defined in terms of textwidth)" #41131

wants to merge 1 commit into from

Conversation

DilumAluthge
Copy link
Member

Reverts #39044

Let's see if CI passes on this PR.

@DilumAluthge DilumAluthge added this to the 1.7 milestone Jun 8, 2021
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Jun 8, 2021
@stevengj
Copy link
Member

stevengj commented Jun 8, 2021

I thought #39044 was passing when it was merged?

@KristofferC
Copy link
Member

KristofferC commented Jun 8, 2021 •
edited
Loading

There is a big CI queue and the status checks don't show up until CI has started. So it is easy to accidentally merge things before CI has even started running on some platforms.

@DilumAluthge
Copy link
Member Author

Once we have moved everything to Buildkite, this will be less of an issue. Buildkite creates a pending (yellow) status as soon as a PR is created/updated, and it stays yellow until all checks have completed.

@DilumAluthge
Copy link
Member Author

Also, this might have been a "semantic conflict", maybe?

That is, maybe the breakage only happens with the combination of both #39044 and a Tar version bump (e.g. #41093)?

@stevengj stevengj mentioned this pull request Jun 8, 2021
Closed
@stevengj
Copy link
Member

stevengj commented Jun 8, 2021

Yes, it looks like it was broken by JuliaIO/Tar.jl@cbcb00b

I have an alternative fix #41133 that should hopefully fix that and similar breakage.

@DilumAluthge DilumAluthge marked this pull request as draft June 8, 2021 17:13
@DilumAluthge
Copy link
Member Author

Nice! Yeah I'd rather we merge #41133 than this PR.

@vtjnash vtjnash added DO NOT MERGE Do not merge this PR! and removed merge me PR is reviewed. Merge when all tests are passing labels Jun 8, 2021
@DilumAluthge DilumAluthge deleted the revert-39044-fix_lpad_rpad_textwidth branch June 8, 2021 19:02
@DilumAluthge DilumAluthge removed the DO NOT MERGE Do not merge this PR! label Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash Awaiting requested review from vtjnash

@stevengj stevengj Awaiting requested review from stevengj

@StefanKarpinski StefanKarpinski Awaiting requested review from StefanKarpinski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
4 participants
@DilumAluthge @stevengj @KristofferC @vtjnash