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

Filter instead of dropwhile to find recompile targets #2047

Closed
wants to merge 1 commit into from

Conversation

filmor
Copy link
Contributor

@filmor filmor commented Apr 4, 2019

This more of an RFC PR, I don't really understand why the dropwhile is used. In our case, we have a (dependency-free) library that is usually in _checkouts while developing that shows up roughly in the middle of the dependency list and results in a rebuild of most dependencies for every compile.

@ferd
Copy link
Collaborator

ferd commented Apr 4, 2019

We recompile all the libs that depend on another one because recompiling it risks impacting features like parse transforms or .hrl files that would bubble up to higher level libraries that depend on them. Just filtering risks breaking builds when, for example, you upgrade lager but all the apps on top are stuck with the old compilation of their parse transform and record definitions.

@ferd ferd closed this Apr 4, 2019
@ferd
Copy link
Collaborator

ferd commented Apr 4, 2019

I do agree however that the needs_compile could possibly be made smarter to know when you would need to bubble the compilation up the tree, but aside from _checkouts this never happens since we don't re-compile dependencies aside from the initial fetch and upgrades.

@filmor
Copy link
Contributor Author

filmor commented May 8, 2019

Well, as said, with the existing sorting, a _checkouts dependency can end up in the middle of the dependency list, and then we indeed do recompile possibly a lot of dependencies. I'll try to fix the sorting.

This pull request was closed.
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