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

Honor clj-kondo's namespace local configuration #388

Merged
merged 10 commits into from
Oct 17, 2022

Conversation

OknoLombarda
Copy link
Contributor

@OknoLombarda OknoLombarda commented Oct 15, 2022

#387

This PR makes refactor-nrepl honor clj-kondo's namespace-local configuration of :unused-namespace linter via config.edn file and also via ns meta when performing clean-ns task. I'll fix/add tests if code is fine

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (run lein do clean, test)
  • Code inlining with mranderson works and tests pass with inlined code (run ./build.sh install -- takes a long time)
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)

Thanks!

Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks on a great path to me - thanks for this one!

src/refactor_nrepl/ns/libspec_allowlist.clj Outdated Show resolved Hide resolved
src/refactor_nrepl/ns/ns_parser.clj Outdated Show resolved Hide resolved
src/refactor_nrepl/ns/libspec_allowlist.clj Outdated Show resolved Hide resolved
Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple final suggestions. Thanks for the quick iteration!

src/refactor_nrepl/ns/ns_parser.clj Outdated Show resolved Hide resolved
src/refactor_nrepl/ns/ns_parser.clj Outdated Show resolved Hide resolved
@OknoLombarda
Copy link
Contributor Author

Well, looks like that's it. Unless you have any other comments, of course

Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost there! one last round and I'll be merging.

CHANGELOG.md Show resolved Hide resolved
src/refactor_nrepl/ns/libspec_allowlist.clj Outdated Show resolved Hide resolved
test/refactor_nrepl/ns/libspec_allowlist_test.clj Outdated Show resolved Hide resolved
.clj-kondo/config.edn Outdated Show resolved Hide resolved
test/refactor_nrepl/ns/libspec_allowlist_test.clj Outdated Show resolved Hide resolved
src/refactor_nrepl/ns/libspec_allowlist.clj Outdated Show resolved Hide resolved
@OknoLombarda
Copy link
Contributor Author

All done. Also, I had to use double quote in tests because somehow with single quote it doesn't get the same form as when it's loaded by parse-ns. I don't understand why, to be honest
https://github.com/OknoLombarda/refactor-nrepl/blob/kondo-ns-local-conf/test/refactor_nrepl/ns/libspec_allowlist_test.clj#L62

Also, I took this snippet from clj-kondo's code, so it should work (I guess such a small portion of code won't cause any licensing issues?)

https://github.com/clj-kondo/clj-kondo/blob/master/src/clj_kondo/impl/analyzer/namespace.clj#L475-L478

@vemv
Copy link
Member

vemv commented Oct 17, 2022

Thanks!

Also, I had to use double quote in tests

It's good. That happens simply because:

user=> (= '{1 1} {1 1})
true

I guess such a small portion of code won't cause any licensing issues?

Yes, it's really small

Copy link
Member

@vemv vemv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking perfect. One last q.

src/refactor_nrepl/ns/pprint.clj Outdated Show resolved Hide resolved
@vemv vemv merged commit 9089875 into clojure-emacs:master Oct 17, 2022
@vemv
Copy link
Member

vemv commented Oct 17, 2022

Thanks yet again!

Will be cutting a release this evening.

Cheers - V

@vemv
Copy link
Member

vemv commented Oct 23, 2022

I have released refactor-nrepl 3.6.0

clj-refactor.el 3.6.0 is been pushed and will be available in a couple hours!

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