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.

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

Add clj-kondo exports and config, fix linting errors, some addt'l fixes #16

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

k13gomez
Copy link
Contributor

@k13gomez k13gomez commented Mar 4, 2024

Hi Chris,
In this PR I've included the following changes:

  • Add clj-kondo exports and config, fix linting errors
  • Remove support for and call to take-last 1-arity, which was not valid.
  • Fix variable arity merge-with, which was not correctly implemented.

Just adding linting some syntax/arity errors became visible and I fixed them, but there could be other issues due to lack of testing, so I'd like to help out to expand testing for the project, to that end I'd like your opinion on the following plan:

  • Swap the tests runner to kaocha, this will allow easy integration with multiple plugins, as well as split the tests by category (performance vs unit tests, etc, see this project for example: https://github.com/k13labs/clara-rules/actions/runs/8090904763/job/22109138392).
  • Add test coverage using clovegage plugin for kaocha, this would only run on tests that do not measure performance, as instrumenting namespaces with coverage naturally reduces performance.
  • Once the codebase is instrumented with coverage reporting, then it becomes easier to add more tests and increase coverage in the areas that are lacking testing.

k13gomez added 3 commits March 3, 2024 21:15

Verified

This commit was signed with the committer’s verified signature.
codeboten Alex Boten

Verified

This commit was signed with the committer’s verified signature.
codeboten Alex Boten

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@cnuernber
Copy link
Owner

Honestly this all sounds great - one thing I noticed is the fixes around the protocol implementation. This implementation isn't complete but I needed a project with a repl to implement 85% or so of the protocol changes. The full changes exist in my cn-master branch of clojure. I may remove the implementation and the tests from this project as cn-master is stable.

@cnuernber cnuernber merged commit 173ca2a into cnuernber:master Mar 4, 2024
@k13gomez k13gomez deleted the lint-and-fixes branch March 4, 2024 15:19
@k13gomez
Copy link
Contributor Author

k13gomez commented Mar 4, 2024

Sounds good, I won't spend much time around the defprotocol namespace, but mainly wanted to get rid of any linting issues to get the project to a point where we could start linting in the GH Actions and fail builds if lint errors are found.

@cnuernber
Copy link
Owner

Sure - makes sense!

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.

None yet

2 participants