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

Remove confusing message since the one right before it says that it won't install #266

Closed
wants to merge 1 commit into from

Conversation

bean5
Copy link

@bean5 bean5 commented May 1, 2015

I saved with Atom, which automatically cleaned up whitespace stuff as well. The main change was known as lines 437-438.

@bean5
Copy link
Author

bean5 commented May 1, 2015

Hmm. Automatic checks are failing. I'm not sure if those are pre-existing. If this is just deprecated code I modified, I suppose if no one wants to update tests accordingly, we might rather deny this request.

I'll admit I'm new to Github checks, so I didn't even know to run them before making the request.

@ferventcoder
Copy link
Member

No worries - have you reviewed the contributing doc?

On Friday, May 1, 2015, Bean notifications@github.com wrote:

Hmm. Automatic checks are failing. I'm not sure if those are pre-existing.
If this is just deprecated code I modified, I suppose if no one wants to
update tests accordingly, we might rather deny this request.

I'll admit I'm new to Github checks, so I didn't even know to run them
before making the request.


Reply to this email directly or view it on GitHub
#266 (comment).

Rob
"Be passionate in all you do"

http://devlicio.us/blogs/rob_reynolds
http://ferventcoder.com
http://twitter.com/ferventcoder

@ferventcoder
Copy link
Member

@bean5 Have you had the opportunity to read and respond to my feedback yet?

@bean5
Copy link
Author

bean5 commented May 25, 2015

I read the contrib document, and accepted the CLA. I amended the commit to contain the pull request number. I still don't know how to run the checks locally to determine whether they were already failing. According to your contrib rules, ideally you don't want to introduce failures. Sorry I don't know how to check locally.

@ferventcoder
Copy link
Member

@bean5 Thanks for following up. The weird part here is that our contrib documentation may need to state that the issue you link to should be an issue and not a pull request. It also seems that you need to fetch from latest and rebase your changes against that so that the build can be completed.

If you want to handle those two things, that would be great! If not I can probably move forward with the removal of the message as a doc commit. Let me know.

@ferventcoder
Copy link
Member

The entire section has been removed so this PR is no longer valid. Thanks for the request though!

@ferventcoder
Copy link
Member

This was invalidated by #300

@bean5
Copy link
Author

bean5 commented Jun 5, 2015

Glad you found that. The rebase/merge on this would have been more of a beast than it was worth it.

@bean5 bean5 deleted the dev branch June 5, 2015 18:41
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.

None yet

2 participants