Closed fgregg closed 5 years ago
Will take a look tomorrow. Thanks.
There's a failed test on py 3.5 if you could take a look. Or we drop 3.5 support and move on.
AssertionError: /home/travis/build/piccolbo/Pweave/tests/weave/tex/FIR_design_verb_REF.tex and /home/travis/build/piccolbo/Pweave/tests/weave/tex/FIR_design_verb.tex differs
try:
$ vimdiff /home/travis/build/piccolbo/Pweave/tests/weave/tex/FIR_design_verb_REF.tex /home/travis/build/piccolbo/Pweave/tests/weave/tex/FIR_design_verb.tex
By the way I normally use black as a formatter because it outputs code that only depend on the parse tree, not any quirks in how it was typed, and puts an end to formatting disputes. It's pep8-compliant, not how I would format things, but better than formatting wars. I see from some reformatting you did you may use something similar. It'd be best to align to keep the diffs small.
Makes sense to me to get tests passing before we start bringing in feature PRs. see #2.
I'm also happy to code formatting scheme you like. After nosetests are passing, would you want to make the existing code base black-compliant, and then add a black linter to the CI?
Sounds like a plan. Even better for me, pre-commit action to format everything, but I need to research it. Linting means another class of failures you have to deal with -- for a good reason but more work. A formatter just does the work for you. I like that better. We are digressing a bit too much on this PR I will open a few issues to focus the discussion.
I will do the formatting. You seem to know what you are doing on the CI. black --check to run a check without changes, --diff to get a diff of what would have to change.
On Thu, Dec 13, 2018 at 6:46 PM Forest Gregg notifications@github.com wrote:
Makes sense to me to get tests passing before we start bringing in feature PRs. see #2 https://github.com/piccolbo/Pweave/pull/2.
I'm also happy to code formatting scheme you like. After nosetests are passing, would you want to make the existing code base black-compliant, and then add a black linter to the CI?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/piccolbo/Pweave/pull/1#issuecomment-447194745, or mute the thread https://github.com/notifications/unsubscribe-auth/AA3RW8D7XJPtvUSIzEG60lPZMPmCwSfXks5u4xD4gaJpZM4ZQymS .
Can you merge master again? Then this should pass. Or I can do it.
I rebased and squashed. Test passing now. @piccolbo
I am against either, but that's a longer story (https://git-scm.com/book/en/v2/Git-Branching-Rebasing , section "perils of rebasing") . Will do for now.
Hi @piccolbo, let me know if you'd like some version of this in your fork. There are other things I'd like to see done with Pweave, and it would be good to find collaborators.