Skip to content

Conversation

@wolever
Copy link
Owner

@wolever wolever commented Apr 14, 2022

This includes #27 and #28

Thank you very kindly to @joaonc for their work on this!

@joaonc
Copy link

joaonc commented Apr 14, 2022

Thanks for picking this up @wolever !

A few things:

  • Having both tox and github actions seems redundant.
  • Were you not insterested in the linting and security checks? black, isort, flake8, mypy, security and bandit.
    I find these to be easy, low-hanging fruit procedures that bring in some benefits.
    Wait, I was looking at a commit, not the whole PR, yeah these are included, cool.

Side note: Some tests were marked as 'fix' b/c they're failing on my machine, which is Windows. I believe there's some encoding that adds \r\n instead of simply \n and tests were failing b/c of that. I was in the process of changing encoding, but not sure it's required..

@joaonc
Copy link

joaonc commented Apr 14, 2022

@joaonc
Copy link

joaonc commented May 24, 2022

@wolever almost there! just click merge 😃 🙏

@milosivanovic
Copy link

Thanks for this! Is there anything else holding it up? Would love to see the sort_dicts option merged in 😃

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.

4 participants