methodical icon indicating copy to clipboard operation
methodical copied to clipboard

defmethod: validate arg count according to arglists

Open markus-wa opened this issue 4 years ago • 2 comments

hi @camsaul I attempted to solve #59

--

Thanks for contributing to Methodical. Before open a pull request, please take a moment to:

  • [x] Ensure the PR follows the Clojure Style Guide.

  • [x] Tests and linters pass. You can run them locally as follows:

    lein test && lein lint
    

    CircleCI will also run these same tests against your PR.

  • [ ] Make sure you've included new tests for any new features or bugfixes. - :warning: I'm not sure how to best do this right now, as we're trying to test a macro that throws an exception, which doesn't seem to be easily testable with standard clojure.test tooling. was hoping you'd be able to guide me to a solution here

  • [ ] New features are documented, or documentation is updated appropriately for any changed features. :warning: not sure if this is needed here?

  • [x] Carefully review your own changes and revert any superfluous ones. (A good example would be moving words in the Markdown documentation to different lines in a way that wouldn't change how the rendered page itself would appear. These sorts of changes make a PR bigger than it needs to be, and, thus, harder to review.)

    Of course, indentation and typo fixes are not covered by this rule and are always appreciated.

  • [x] Include a detailed explanation of what changes you're making and why you've made them. This will help me understand what's going on while we review it.

Once you've done all that, open a PR! Make sure to at-mention @camsaul in the PR description. Otherwise I won't get an email about it and might not get review it right away. :)

Thanks for your contribution!

markus-wa avatar Oct 22 '21 17:10 markus-wa

maybe for code coverage we could extract a function and use this hack? https://stackoverflow.com/a/37471356 what's your thoughts on that?

markus-wa avatar Oct 22 '21 17:10 markus-wa

re testing: I'm not sure how to best do this right now, as we're trying to test a macro that throws an exception, which doesn't seem to be easily testable with standard clojure.test tooling. was hoping you'd be able to guide me to a solution here

markus-wa avatar Feb 10 '22 08:02 markus-wa

Thank you for taking the time to work on this PR.

I ended up implementing this in #130 -- it ended up being quite a bit more complicated than I originally thought, since we have to be able to handle & rest (vararg) arities.

camsaul avatar Sep 09 '22 23:09 camsaul