Bug #36148

Checkouts do not belong to the changeset

Added by Christian Trabold about 1 year ago. Updated about 1 year ago.

Status:Closed Start date:2012-04-16
Priority:Must have Due date:
Assignee:- % Done:

100%

Category:-
Target version:- Estimated time:4.00 hours
Votes: 0

Description

Jenkins reports failure with https://review.typo3.org/#/c/9719/ but obviously https://ci.typo3.org/job/typo3-v4-core-gerrit/3735/ does not belong to the changeset.

Please check if Jenkins gets and uses the correct ENV-Variable from Gerrit.

History

Updated by Christian Trabold about 1 year ago

Stefano: IIRC you are the last one working on the Gerrit trigger (reject the ticket to me, if I'm wrong).

Could you please take a look on the setup and tell us what could be the reason?

Thanks!

Updated by Christian Trabold about 1 year ago

This code sniffer error fails the build:

https://ci.typo3.org/view/TYPO3/view/Core/job/typo3-v4-core-gerrit/3763/checkstyleResult/source.1147/#191

As the mentioned file is not part of the commit change, chances are that class.tslib_content_media.php had been introduced by an earlier commit and now gets catched by phpcs.

RFC for Solution

The committer should fix the file class.tslib_content_media.php in the commit so phpcs is happy.

Updated by Steffen Müller about 1 year ago

The CGL issue in class.tslib_content_media.php needs to be fixed in a separate changeset. Would that help?

Updated by Christian Trabold about 1 year ago

  • Status changed from Accepted to Under Review
  • % Done changed from 0 to 80

Steffen Müller wrote:

The CGL issue in class.tslib_content_media.php needs to be fixed in a separate changeset. Would that help?

After thinking that over, I think there's no need for doing that.

I added a potential fix for the checkstyle script: The check should only consider files that were changed by the last commit.

Checkstyle currently checks whole TYPO3 core, which can lead to "inconsistent" changesets and blockers.

Updated by Steffen Müller about 1 year ago

checkstyle should only check the lines of the changeset itself, not the whole file.

CGL and functional changes should not be mixed, because it makes review harder. Of course CGL should be applied to all changed or new lines.

Updated by Christian Trabold about 1 year ago

  • Status changed from Under Review to Resolved
  • % Done changed from 80 to 100

My patch fixed the issue.

Steffen Müller wrote:

checkstyle should only check the lines of the changeset itself, not the whole file.

CGL and functional changes should not be mixed, because it makes review harder. Of course CGL should be applied to all changed or new lines.

Thanks for your feedback. Should be discussed with CGL-Team IMO.

Updated by Christian Trabold about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF