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.

Sign up for GitHub

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

Jump to bottom

PyBuilder install_dependencies behavior should mirror #340 #347

Merged
merged 1 commit into from
Apr 15, 2016
Merged

PyBuilder install_dependencies behavior should mirror #340 #347

merged 1 commit into from
Apr 15, 2016

Conversation

arcivanov
Copy link
Member

  1. Got rid of Mockito entirely. This resulted in finding a huge bug in reactor tests whereas the plugin loader in the actual (not mocked) reactor was substituted with a mock plugin loader affecting further reactor behavior. https://github.com/pybuilder/pybuilder/pull/347/files#diff-9584cce2b40001f24aa966f2329c0bc7R50 https://github.com/pybuilder/pybuilder/pull/347/files#diff-9584cce2b40001f24aa966f2329c0bc7R421
  2. We use Unittest.Mock where it is available and Mock elsewhere. Dependencies are adjusted accordingly.
  3. All of the PIP internal stuff is moved into pip_common (to expose Version and SpecifierSet) while pip_utils contains PIP-related utilities. https://github.com/pybuilder/pybuilder/pull/347/files#diff-3f7930768a016caa358fe51c630f08cfR1
  4. Corollary to self-updating (since we use PIP internals) is reloading of PIP post-update to allow incompatible migrations from 6 to 8 etc. https://github.com/pybuilder/pybuilder/pull/347/files#diff-001479a18f1db7b9cde893d99c6734f1R133
  5. Logic related to what to install is concentrated here: https://github.com/pybuilder/pybuilder/pull/347/files#diff-001479a18f1db7b9cde893d99c6734f1R136
  6. Logic related to how to install (if there are individual install targets or URL install causing force-reinstall must install separately, otherwise can batch) is here:
    https://github.com/pybuilder/pybuilder/pull/347/files#diff-001479a18f1db7b9cde893d99c6734f1R106

fixes #346, connected to #346

@arcivanov arcivanov self-assigned this Apr 8, 2016
@arcivanov arcivanov force-pushed the issue_346 branch 3 times, most recently from 537d1af to 8064612 Compare April 11, 2016 03:59
@mriehl
Copy link
Member

mriehl commented Apr 11, 2016

@arcivanov is this ready for review or are you still tinkering with it?

@arcivanov arcivanov force-pushed the issue_346 branch 2 times, most recently from bf67c32 to ae9a3ad Compare April 11, 2016 20:13
@arcivanov
Copy link
Member Author

@mriehl still tinkering, hence the WIP label

@arcivanov arcivanov force-pushed the issue_346 branch 4 times, most recently from d0ce2ec to d9c29ac Compare April 12, 2016 00:01
@coveralls
Copy link

Coverage Status

Coverage increased (+1.6%) to 72.063% when pulling d9c29ac on arcivanov:issue_346 into bc3b670 on pybuilder:master.

@arcivanov
Copy link
Member Author

@mriehl ready for preliminary review

@arcivanov
Copy link
Member Author

@mriehl @esc bump

mriehl
mriehl reviewed Apr 14, 2016
View reviewed changes
src/main/python/pybuilder/plugins/python/install_dependencies_plugin.py
" from %s" % url if url else "")
log_file = project.expand_path("$dir_install_logs", dependency.name)
log_file = re.sub(r'<|>|=', '_', log_file)
def install_dependency(logger, project, dependencies):
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be install_dependencies now

Copy link
Member Author

Choose a reason for hiding this comment

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

backward compatibility. I'll alias it.

Copy link
Member Author

@arcivanov arcivanov Apr 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

can't alias it, conflicts with task function. at any rate it supports 1 to many

@mriehl
Copy link
Member

mriehl commented Apr 14, 2016

Good stuff, and nice find on the mock.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 72.207% when pulling eb036f4 on arcivanov:issue_346 into 157420e on pybuilder:master.

@arcivanov arcivanov merged commit c6f9689 into pybuilder:master Apr 15, 2016
@arcivanov arcivanov deleted the issue_346 branch April 15, 2016 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@arcivanov arcivanov

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PyBuilder install_dependencies behavior should mirror #340
3 participants
@arcivanov @mriehl @coveralls