Skip to content

Test that install actually works #918

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

Closed
wants to merge 1 commit into from
Closed

Test that install actually works #918

wants to merge 1 commit into from

Conversation

o11c
Copy link
Contributor

@o11c o11c commented Oct 13, 2015

Closes #915 which is critical.

Remaining issues: #917, #732

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 13, 2015

Sorry, I can't review pull requests that change things all over the place without any documentation on what is the purpose of this. Try again by splitting this into atomic changes that address one issue at a time with the smallest reasonable delta -- either a refactoring, a bug fix or a functionality change, and document what each change does. I can't figure out why you are changing things in the tests, for example.

@JukkaL JukkaL closed this Oct 13, 2015
@o11c
Copy link
Contributor Author

o11c commented Oct 13, 2015

Good luck shipping a program that is completely broken then.

I'll just tell everybody to use my fork instead.

@o11c
Copy link
Contributor Author

o11c commented Oct 13, 2015

Also, you're not one to talk at all about documentation of internals.

@o11c o11c mentioned this pull request Oct 13, 2015
@o11c o11c deleted the setup branch October 13, 2015 05:19
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 13, 2015

Please file an issue if something is unclear about the internals. Complaining about it in comments without explaining what is unclear does not improve things.

I can't merge pull requests if I don't understand them, no matter how important the fix is. Are you going to document your PR?

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