Skip to content

Conversation

@mohamed82008
Copy link
Contributor

This PR doesn't introduce functional changes, it just moves some code around and puts them in files. This was part of #115.

@codecov
Copy link

codecov bot commented May 13, 2020

Codecov Report

Merging #116 into master will not change coverage.
The diff coverage is 83.38%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #116   +/-   ##
=======================================
  Coverage   74.83%   74.83%           
=======================================
  Files          14       17    +3     
  Lines         906      906           
=======================================
  Hits          678      678           
  Misses        228      228           
Impacted Files Coverage Δ
src/DynamicPPL.jl 100.00% <ø> (ø)
src/varinfo/types.jl 70.58% <70.58%> (ø)
src/varinfo/utils.jl 82.94% <82.94%> (ø)
src/varinfo/linking.jl 88.46% <88.46%> (ø)
src/varinfo/indexing.jl 92.10% <92.10%> (ø)

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 f74e039...5210b19. Read the comment docs.

@mohamed82008 mohamed82008 merged commit 7ab6615 into master May 13, 2020
@bors bors bot deleted the mt/reorganize branch May 13, 2020 20:15
@yebai
Copy link
Member

yebai commented May 13, 2020

Changes in this PR are reverted. We discussed how to organise VarInfo a while ago, I still think a single file-based VarInfo with sections is easier to review and maintain. We can revisit this consensus in the future if necessary, but better to have some discussions before changing it.

@mohamed82008
Copy link
Contributor Author

Sure we can revert this if you are really against it. But I am finding it difficult working on the varinfo.jl file as it is right now. It's a gigantic file that does a lot of things which can be grouped together nicely in files. I don't see why that's a bad thing.

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