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

Make pipsi a package and make the inline scripts into files #110

Merged
merged 5 commits into from
Nov 10, 2017
Merged

Make pipsi a package and make the inline scripts into files #110

merged 5 commits into from
Nov 10, 2017

Conversation

pfmoore
Copy link
Contributor

@pfmoore pfmoore commented Oct 11, 2017

No description provided.

@pfmoore pfmoore mentioned this pull request Oct 11, 2017
Closed
RonnyPfannschmidt
RonnyPfannschmidt approved these changes Oct 11, 2017
View reviewed changes
Copy link
Contributor

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

looks good, will try it later today

@pfmoore
Copy link
Contributor Author

pfmoore commented Oct 25, 2017

I'm not sure if the test failures here are problems with the PR. Some point to code I added, but others seem to be the "special characters" errors that appear to be hitting all PRs at the moment.

@pfmoore
Copy link
Contributor Author

pfmoore commented Nov 6, 2017

Rebased to pick up the test fixes in master

@pfmoore
Copy link
Contributor Author

pfmoore commented Nov 6, 2017

OK, looks like there are actual problems I need to fix...

@pfmoore
Copy link
Contributor Author

pfmoore commented Nov 10, 2017

Tests fixed, although the final fix just disables one of the "simple install" tests that uses pipsi as a package to install in a way that clashes with having a local directory called pipsi. I'm not particularly happy with doing that, but I can't see a better fix without some more significant surgery on how the test suite works (which I don't have the time to do right now).

RonnyPfannschmidt
RonnyPfannschmidt reviewed Nov 10, 2017
View reviewed changes
testing/test_repo.py Outdated
@@ -11,7 +11,7 @@ def repo(home, bin):

@pytest.mark.parametrize('package, glob', [
('grin', 'grin*'),
('pipsi', 'pipsi*'),
# ('pipsi', 'pipsi*'),
Copy link
Contributor

Choose a reason for hiding this comment

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

in order to keep track, please make this an xfail with a reason reference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will do. It's a bit fiddly to just mark one set of parameters as xfail. I think I have the correct incantation, but if it fails again I'll work out how to fix it.

@RonnyPfannschmidt
Copy link
Contributor

lets make an issue to track that

@pfmoore
Copy link
Contributor Author

pfmoore commented Nov 10, 2017

#115 created for this.

@RonnyPfannschmidt
Copy link
Contributor

nicely done, thanks

@RonnyPfannschmidt RonnyPfannschmidt merged commit 97c4366 into mitsuhiko:master Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RonnyPfannschmidt RonnyPfannschmidt RonnyPfannschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pfmoore @RonnyPfannschmidt