Skip to content

Conversation

@altendky
Copy link
Contributor

@altendky altendky commented Mar 26, 2021

Draft for:

This looks like a nasty format-it-in-my-style PR. I promise you that is not the intent. :] The issue is that the original use of textwrap.dedent() was stripping the leading tabs. That is _all_ I hope to fix here. There are certainly several other ways this could be written. I happened to pick this way. You are welcome to correct it, provide a separate fix, or let me know how you want it to look and I'll do the grunt work. I'll be happy with whatever solution.

This could be argued to relate to #14 where pylddwrap would become sensitive to the indentation. I thought it made sense as a separate PR since it fixes an existing and independent inaccuracy in the tests.

Side note, would you be interested in a GitHub Actions testing workflow? I'd be happy to write that up for you. (edit: I just went for it #16)

@altendky
Copy link
Contributor Author

Hmm, I'll mark this as draft as well in case I find more related bits to tweak.

@altendky altendky marked this pull request as draft March 26, 2021 14:47
@altendky altendky mentioned this pull request Mar 26, 2021
4 tasks
@altendky altendky marked this pull request as ready for review April 12, 2021 19:20
@altendky
Copy link
Contributor Author

FWIW, I don't like how the strings with leading tabs are written. I considered several other options and all were gross. If there's something you like, or dislike less, that retains the tabs... lemme know and I'll write them that way.

@altendky
Copy link
Contributor Author

altendky commented May 6, 2021

@mristin, this one is ready for consideration as well.

@mristin
Copy link
Collaborator

mristin commented May 7, 2021

LGTM, thanks! Should I merge?

@altendky
Copy link
Contributor Author

altendky commented May 7, 2021

Go for it. Thanks.

@mristin mristin merged commit 03ddbf8 into Parquery:master May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants