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

Lower bandwidth at startup #264

Merged
merged 8 commits into from
Sep 17, 2019

Conversation

paulrd
Copy link
Contributor

@paulrd paulrd commented Aug 1, 2019

Before submitting a PR make sure the following things have been done (and denote this
by checking the relevant checkboxes):

  • 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!

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

This addresses #263. I couldn't get ./build install to run without errors even before I made these changes. Let's see if circleci passes.

Copy link
Member

@expez expez left a comment

Choose a reason for hiding this comment

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

Great job on this! 👍

src/refactor_nrepl/artifacts.clj Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

Cool I'll make those changes. I'm having a hard time testing this in emacs, how do you go about testing this within emacs? When I do a lein install, I don't get the same behavior as when I run in the a separate lein repl (with the appropriate profiles). Maybe I should try renaming the project and deploy to my personal clojars to get it to work.

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

runnig ./build install i get the following:
Exception in thread "main" java.lang.ClassNotFoundException: javax.xml.bind.DatatypeConverter, compiling:(cljs/util.cljc:1:1)

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

I think I know my issue with build.sh, I keep forgetting about the proper profile.

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

Testing in emacs is giving me different behavior. It's downloading on every invocation of jack-in. It is getting the gzipped version at least :-).

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

Is make inline-deps always required before I do lein install?

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

ah! :force true is set! why!?

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

Ah, ok it's not a bug. The default custom variable cljr-populate-artifact-cache-on-startup is true. Changing it to nil fixes the issue. I might open an issue there just as a reminder.

@paulrd
Copy link
Contributor Author

paulrd commented Aug 1, 2019

One more thing I should change I should either keep the map sorted or resort it when loading from cache file.

Copy link
Member

@expez expez left a comment

Choose a reason for hiding this comment

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

Looks good other than a final nitpick! 👍

src/refactor_nrepl/artifacts.clj Outdated Show resolved Hide resolved
@bbatsov bbatsov merged commit bb4d730 into clojure-emacs:master Sep 17, 2019
bbatsov pushed a commit that referenced this pull request Sep 17, 2019
@bbatsov
Copy link
Member

bbatsov commented Sep 17, 2019

Thanks!

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