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

Introduce :insert-newline-after-require #303

Merged
merged 3 commits into from
Jun 27, 2021

Conversation

vemv
Copy link
Member

@vemv vemv commented Jun 27, 2021

  • Upgrade Eastwood, clj-kondo
  • Makefile: also use -user profile
    • This increases reproducibility while developing the project.
  • Introduce :insert-newline-after-require option
  • 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)

@@ -11,4 +11,5 @@
[transport :as transport]]
[org.httpkit.client :as http]
[refactor-nrepl.externs :refer [add-dependencies]])
(:import java.util.Date))
(:import
Copy link
Member Author

Choose a reason for hiding this comment

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

I made the treatment of :import homogeneous relative to that of :require, as most likely users never want an asymmetry.

(It's also what https://github.com/bbatsov/clojure-style-guide/tree/e6cb6dab0925bcfabcd4ec96a2f69f41bf9c698e#line-break-ns-declaration suggests in its example)

@bbatsov bbatsov merged commit 9fee2ed into clojure-emacs:master Jun 27, 2021
@bbatsov
Copy link
Member

bbatsov commented Jun 27, 2021

Great work! Thanks!

@expez
Copy link
Member

expez commented Jun 27, 2021

Great work @vemv! Makes total sense to give :import the same treatment :)

You might already be on this, but we also need a defvar and a changelog entry in clj-refactor.el to toggle this on/off and then send it to the middleware. We already have a setup for this so should be fairly easy.

@vemv
Copy link
Member Author

vemv commented Jun 27, 2021

You might already be on this, but we also need a defvar and a changelog entry in clj-refactor.el to toggle this on/off and then send it to the middleware.

Yeah I figured so! Later today

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.

3 participants