Skip to content

Conversation

@cstjean
Copy link
Collaborator

@cstjean cstjean commented Sep 9, 2017

As suggested in #33 (comment)

Issues:

  • I like revise(module) as a verb, but it's not entirely obvious what it's going to do. forcerevise(module)?
  • Both here and in TraceCalls.jl, I get the module files from parse_pkg_files, which seems a tad wasteful given that those files were already parsed. We could maintain module2files.
  • revise(module) triggers a lot of redefinition warnings. In ClobberingReload I use this code. We could move over scrub_stderr to Revise.jl

This is a work-around for #20 and #31, I'll leave you to decide if you want to close these issues.

@cstjean cstjean requested review from timholy and removed request for timholy September 12, 2017 01:54
@cstjean
Copy link
Collaborator Author

cstjean commented Sep 12, 2017

Sorry - accidental click.

@timholy
Copy link
Owner

timholy commented Sep 12, 2017

If you're up for the module2files route, I think I'd prefer that.

@cstjean cstjean mentioned this pull request Sep 15, 2017
@cstjean cstjean force-pushed the pull-request/2f01000e branch from 2f01000 to 9c9802f Compare September 18, 2017 13:29
@codecov
Copy link

codecov bot commented Sep 18, 2017

Codecov Report

Merging #41 into master will increase coverage by 0.45%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #41      +/-   ##
==========================================
+ Coverage   83.16%   83.61%   +0.45%     
==========================================
  Files           1        1              
  Lines         297      293       -4     
==========================================
- Hits          247      245       -2     
+ Misses         50       48       -2
Impacted Files Coverage Δ
src/Revise.jl 83.61% <100%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00cc2da...9c9802f. Read the comment docs.

@timholy timholy merged commit cf06d0d into timholy:master Sep 18, 2017
@timholy
Copy link
Owner

timholy commented Sep 18, 2017

Thanks a bunch!

The OSX tests routinely fail at the "cleanup" step. I don't suppose you can reproduce that yourself, can you? If so, does adding a sleep or something fix it?

@cstjean
Copy link
Collaborator Author

cstjean commented Sep 18, 2017

I've seen it a few times, I'll try to fix it.

This was referenced Oct 8, 2017
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