Test new3 #189

Closed
wants to merge 3 commits into
from

Projects

None yet

6 participants

@beberlei
Owner

last test i hope :D

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DCOM-113

Oh btw, I just (automatically) realized that you are not creating this pull request against the master branch.

Unless there are good reasons for this, i would suggest to close and rebase the Pull Request against master and then create it again. Sorry!

@beberlei beberlei closed this Sep 24, 2012

Oh btw, I just (automatically) realized that you are not creating this pull request against the master branch.

Unless there are good reasons for this, i would suggest to close and rebase the Pull Request against master and then create it again. Sorry!

Welcome doctrinebot =)

Great stuff here. I am a bot as well ..... lol

Contributor
till commented Sep 25, 2012

Maybe you can use the commit status API with your bot?

Member
stof commented Sep 25, 2012

@till I don't think it is a good idea:

  • @doctrinebot is not about giving a status for the PR but giving some feedback to the guy opening the PR
  • if @doctrinebot updates the status after Travis, it hides the Travis status, which would be useful for us.
Contributor
till commented Sep 25, 2012

You can't have multiple? Weird. That's gonna be fun for a lot of services.

Member
stof commented Sep 25, 2012

@till you can have as many statuses as you want for each commit. But the PR page only shows the most recent one: https://github.com/blog/1227-commit-status-api

Member
stof commented Sep 25, 2012

hmm, maybe it should use the status API only in the case of a wrong target, so that the merge button shows a warning.

Contributor
till commented Sep 25, 2012

Yeah, that's what I mean – to indicate if this merges to master.

till
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

On Tuesday, September 25, 2012 at 2:12 PM, Christophe Coevoet wrote:

hmm, maybe it should use the status API only in the case of a wrong target, so that the merge button shows a warning.


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

Owner

Only showing the last commit status? That sucks big time :( I was about to create a PHPCS service for commit diffs.

Member
stof commented Sep 25, 2012

@beberlei you mean, like one of the features implemented in @schmittjoh's tool ?

Owner

sort of, I also have working code for this, only need to hook it into the commit status API. I don't want to comment spam and link to a static page with the violations instead.

Member
stof commented Sep 25, 2012

@beberlei Its tool now has a page with the violations too. I'm not sure it is commenting on the PR anymore for all of the changes (it puts one comment with the link, which could probably be replaced by the status api).

Member

It actually uses the status API now, I've updated it a few days ago.

see http://jmsyst.com/reviews/28d69601-7920-4f75-9877-8b36981678dc for an example

Owner

@schmittjoh aah! you sir, need help in the marketing department ;-)

@stof stof deleted the TestNew3 branch Dec 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment