You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by SudharmaJain <gi...@git.apache.org> on 2015/12/14 12:46:30 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
GitHub user SudharmaJain opened a pull request:
https://github.com/apache/cloudstack/pull/1243
CLOUDSTACK-9166:Build failed in Jenkins: cloudstack-rat-master #7038
With my last PR#1196, I missed to place the license disclaimer into CitrixHelperTest.java. Updated the file with the disclaimer.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SudharmaJain/cloudstack cs-9166
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1243.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1243
----
commit 601f2dba24f7665d295627e3b2a2eb6f4aae9f77
Author: SudharmaJain <su...@citrix.com>
Date: 2015-12-14T11:39:12Z
CLOUDSTACK-9166:Build failed in Jenkins: cloudstack-rat-master #7038
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164474404
@rafaelweingartner
It's not about throwing stones, it's about the process.
Simply put: 2 LGTM, and depending on the PR at least 1 with integration tests executed. In this case, integration tests were not needed. It is a very simple PR which could have been reviewed very quickly and none of this would have happened. The whole storm behind it is that we have to keep the process and avoid that something like that happens with more complex code - as it happened just after 4.6.0.
Let's save some typing and time and not start a discussion about the obvious.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164425397
@DaanHoogland Could you please explain why you merged this?
We have a policy stating **2 (TWO)** persons will have to review a PR. One code review and one running tests. **At all times**.
How trivial it is doesn't matter.
I therefore want it to be reverted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/1243
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164417836
argh, thanks @SudharmaJain LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164489480
Don't go here. this is about licensing. We have a duty do do this with
extreme prejudice.
On Mon, Dec 14, 2015 at 5:00 PM, borisroman <no...@github.com>
wrote:
> @SudharmaJain <https://github.com/SudharmaJain> Could you please open a
> new PR so it can be merged with reviews.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/cloudstack/pull/1243#issuecomment-164475560>.
>
--
Daan
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164471811
@Wilderrodrigues, @borisroman I agree with you that we cannot merge PRs without two(2) LGTMs and integration tests to check if nothing was broken. @DaanHoogland should have not merged this PR without the second LGTM. However, before he had merged it, I reviewed the PR but have not given LGTM because normally these simple PRs a lot of folks appear to give LGTM, my bad for that. Additionally, I got lost when I checked the right way to include the Apache license, I notice in same classes we use /** * * **/ blocks, in others we use //, while thinking about it something else called my attention and I forgot to give the LGTM.
Before, throwing some stones, I would first check why PR #1196 was merged without the Apache license. I noticed LGTMs there; maybe they were giving without proper review? So, here comes a question, should we revert this one that fixes a build that was broken by #1196, or should we revert the #1196 and fix it before merging it again? Or we can let it go and learn the lesson that should not be repeated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by borisroman <gi...@git.apache.org>.
Github user borisroman commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164475560
@SudharmaJain Could you please open a new PR so it can be merged with reviews.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164494786
I truly believe we need a meeting to discuss this. I understand the hole licensing thing, but having a PR open to review is already a prove that the issue is being dealt with.
It's your call... I'm out of this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9166:Build failed in Jenkins: ...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/1243#issuecomment-164467579
+1 for reverting it. And I don't even care if the RAT is broken after the rever. Reasons are obvious, but we have to stick to the 2 LGTM otherwise the project will become the hell it was a few months ago.
I have been to hell... not funny! Do not want to go back.
For a such good reason, as stated in this PR, a simply ping someone would make it get the 2 LGTM and the merge would be fine.
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---