Skip to content

Improvements to blocker handling in fine-grained incremental mode, etc. #4292

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

Merged
merged 24 commits into from
Dec 5, 2017

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Nov 29, 2017

Handle various additional cases of blocking errors. This also contains a fair
amount of refactoring.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

LG, just two nits.

('remaining', List[Tuple[str, str]])])

# The result of update_single_isolated when there is a blocking error. Items
# are similar to NormalUpdate (but there are fewer).
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it makes sense to move remaining up in the list for NormalUpdate so that the first three items always have the same meaning?

@@ -316,7 +372,9 @@ def delete_module(module_id: str,
# TODO: Deletion of a package
# TODO: Remove deps for the module (this only affects memory use, not correctness)
new_graph = graph.copy()
del new_graph[module_id]
if module_id in new_graph:
# TODO: Why would this ever be reached?
Copy link
Member

Choose a reason for hiding this comment

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

What if you replace it with assert module_id not in new_graph?

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Go ahead and merge when you're ready.

@JukkaL JukkaL merged commit 97de77b into master Dec 5, 2017
@gvanrossum gvanrossum deleted the fine-grained-misc branch December 5, 2017 18:35
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