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

Upgrade dependencies #22

Merged
merged 3 commits into from
Oct 25, 2016
Merged

Conversation

jokimaki
Copy link
Contributor

Added test case for a .cljc file change and reader conditional. Reproduces #20 and we can see that after upgrading tools.namespace to 0.2.11 the test passes.

Added profiles for Clojure 1.8.0 and 1.9.0-alpha13.

lein ancient reported that commons-io and org.clojure/java.classpath were out of date too, so I upgraded them as well.

@weavejester
Copy link
Owner

Thanks for the patch. Can you split this into three commits:

Update dependencies
Add test profiles for Clojure 1.8 and 1.9
Add test case for .cljc files

Also can you wrap line 59 in the test file into two lines? It's a little long.

@jokimaki
Copy link
Contributor Author

Sure thing, done!

@weavejester
Copy link
Owner

Almost perfect, you just forgot to change "Upgraded dependencies" to "Update dependencies". I follow the seven rules for commit messages, so I use the imperative mood rather than the past tense (e.g. update rather than updated).

@jokimaki
Copy link
Contributor Author

Good catch, I changed that too now.

@weavejester weavejester merged commit 257e2c6 into weavejester:master Oct 25, 2016
@jokimaki jokimaki deleted the upgrade-deps branch October 30, 2016 20:06
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