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

Track a namespace's dependencies on non-Clojure files #24

Merged
merged 2 commits into from
Dec 9, 2018

Conversation

luontola
Copy link
Contributor

@luontola luontola commented Nov 1, 2018

This PR implements issue #23.

If you add :resource-deps metadata to a namespace declaration and list there all the static resources which the namespace depends on, then that namespace will be reloaded whenever one of those resources is changed.

An example usage with Luminus conman would look like this:

(ns ^{:resource-deps ["sql/queries.sql"]} rems.db.core
  (:require [conman.core :as conman]))
...
(conman/bind-connection *db* "sql/queries.sql")

Additionally Ring's wrap-reload may need to be configured to find the resources directory:

(-> handler
    (wrap-reload {:dirs ["src" "resources"]}))

This PR is also available as a published binary; add the following dependency and exclude the original ns-tracker.

[org.clojars.luontola/ns-tracker "0.3.1-patch1"]
[ring/ring-devel "1.6.3" :exclusions [ns-tracker]]

luontola added a commit to CSCfi/rems that referenced this pull request Nov 1, 2018
Copy link
Owner

@weavejester weavejester left a comment

Choose a reason for hiding this comment

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

Thanks for the patch! I have some comments/corrections/suggestions.

luontola added a commit to CSCfi/rems that referenced this pull request Nov 6, 2018
@luontola
Copy link
Contributor Author

luontola commented Nov 7, 2018

That should be all the discussed changes covered. The readme still needs to be updated. I'll do it later today.

@luontola
Copy link
Contributor Author

luontola commented Nov 7, 2018

That's all that comes to my mind. Do you have any more feedback? I've published this version also as [org.clojars.luontola/ns-tracker "0.3.1-patch2"]

@luontola
Copy link
Contributor Author

luontola commented Nov 8, 2018

Is that all?

@weavejester
Copy link
Owner

Thanks, that looks all fine. Can you wrap the source code at 80 characters, and the README at 72 characters?

@weavejester
Copy link
Owner

I'm afraid your code reformat makes it really quite hard to see what's changed, as you've altered a lot of lines that have nothing to do with the PR. I realize the codebase is old, but can you limit your changes to the code that has been altered, and we can save a global format for another PR?

@luontola
Copy link
Contributor Author

👌

@luontola
Copy link
Contributor Author

The full reformat is in branch https://github.com/luontola/ns-tracker/tree/code-reformat

@weavejester
Copy link
Owner

Thanks, this looks great. Can you squash your commits down? Once there's a clean history I think we can merge.

@luontola
Copy link
Contributor Author

Squashed. One commit with the feature and another for global code reformat.

@weavejester
Copy link
Owner

Sorry for the delay in getting around to reviewing this. Everything looks okay. Can you change the commit messages to follow these seven rules, and then it should be good to merge.

Some libraries such as HugSQL define functions in a namespace based on
the contents of a static resource file. When that file is changed, the
namespace needs to be reloaded to reflect the changes.

This change adds support for a :ns-tracker/resource-deps metadata in the
namespace declaration for listing the resources that the namespace
depends on, to facilitate reloading the namespace when one of the
resources changes.

Resolves: weavejester#23
@luontola
Copy link
Contributor Author

luontola commented Dec 3, 2018

Commit messages changed. 👌

@weavejester weavejester merged commit 0785c37 into weavejester:master Dec 9, 2018
@luontola
Copy link
Contributor Author

Since there is not yet a new release of ns-tracker, I've published an unofficial build of the current master branch:

[org.clojars.luontola/ns-tracker "0.3.1-patch3"]
[ring/ring-devel "1.6.3" :exclusions [ns-tracker]]

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