Improve targeting of reverted commits

nlisgo commented 9 years ago

Firstly, this is a great resource that rightly draws attention to the people that are making a significant contribution to the Drupal 8 project. Thanks.

This pull request addresses an issue that is causing some people to not be acknowledged as contributors.

It is quite common for a commit to be reverted and the d.o issue to remain open and a future patch gets accepted.

For instance #2089397 http://cgit.drupalcode.org/drupal/log/?qt=grep&q=%232089397

Original commit: Issue #2089397 by swentel, Wim Leers: Fixed Double 'Quick edit' contextual link. () Revert of original commit: Revert "Issue #2089397 by swentel, Wim Leers: Fixed Double 'Quick edit' contextual link." Followup commit: Issue #2089397 by jessebeach, swentel, Wim Leers: Fixed Double 'Quick edit' contextual link.

The code as it stands ignores the followup commit instead of the original commit. So jessebeach does not receive acknowledgement for this issue.

At the time of preparing this pull request this fix will allow the script to acknowledge the efforts of 2726 contributors as opposed to 2718 without the fix. And jessebeach will have registered 151 commits as opposed to 149.

I have a branch that I have been using on my fork to help me debug around this issue: https://github.com/nlisgo/drupalcores/tree/experimental/improve-targeting-reverts

It helped me to compare which commits were being reverted before and after the fix.

nlisgo commented 9 years ago

The failure seems to be due to the minifyhtml step rather than this code. I can recreate this on the master branch too. Can you verify?

If you amend the code in the gulpfile.js as below then the problem goes away:

// The whole shebang
gulp.task('default', function(callback) {
  runSequence(['clean', 'bower', 'drupalcore'],
              ['buildcontributors', 'buildcompanies', 'buildjson', 'javascripts', 'images', 'sass'],
              'usemin',
              // 'minifyhtml',
              callback);
});

// Run contributors only, because companies can take ages the first time
gulp.task('contributors', function(callback) {
  runSequence(['clean', 'bower', 'drupalcore'],
              ['buildcontributors', 'buildjson', 'javascripts', 'images', 'sass'],
              'usemin',
              // 'minifyhtml',
              callback);
});

nlisgo commented 9 years ago

I have a pull request submitted to the gulp-minify-html project https://github.com/jonathanepollack/gulp-minify-html/pull/19

Until this is resolved then the minifyhtml step is broken for drupalcores.

lewisnyman commented 9 years ago

I've fixed the error in https://github.com/lauriii/drupalcores/commit/0a4d2b499974c89977771602134369ce3cdfc42d - this PR should pass now

lauriii / drupalcores

Improve targeting of reverted commits #53