Merge pull request #4640 from Flamefire/error-new-pr

improve error message for `--new-pr` when missing commit message

easybuild/tools/github.py

@@ -1037,8 +1037,15 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_
     elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and all(file_info['new']):
         commit_msg = "adding easyblocks: %s" % ', '.join(os.path.basename(p) for p in file_info['paths_in_repo'])
     else:
+        msg = ''
+        modified_files = [os.path.basename(p) for new, p in zip(file_info['new'], file_info['paths_in_repo'])
+                          if not new]
+        if modified_files:
+            msg += '\nModified: ' + ', '.join(modified_files)
+        if paths['files_to_delete']:
+            msg += '\nDeleted: ' + ', '.join(paths['files_to_delete'])
         raise EasyBuildError("A meaningful commit message must be specified via --pr-commit-msg when "
-                             "modifying/deleting files or targeting the framework repo.")
+                             "modifying/deleting files or targeting the framework repo." + msg)
 
     # figure out to which software name patches relate, and copy them to the right place
     if paths['patch_files']:
@@ -1078,8 +1085,8 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_
         # only consider new easyconfig files for dependencies (not updated ones)
         for idx in range(len(all_dep_info['ecs'])):
             if all_dep_info['new'][idx]:
-                for key in dep_info:
-                    dep_info[key].append(all_dep_info[key][idx])
+                for key, info in dep_info.items():
+                    info.append(all_dep_info[key][idx])
 
     # checkout target branch
     if pr_branch is None:
@@ -1692,7 +1699,7 @@ def post_pr_labels(pr, labels):
 
         pr_url = g.repos[pr_target_account][pr_target_repo].issues[pr]
         try:
-            status, data = pr_url.labels.post(body=labels)
+            status, _ = pr_url.labels.post(body=labels)
             if status == HTTP_STATUS_OK:
                 print_msg("Added labels %s to PR#%s" % (', '.join(labels), pr), log=_log, prefix=False)
                 return True
@@ -1991,9 +1998,9 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None):
                     patch = patch[0]
                 elif isinstance(patch, dict):
                     patch_info = {}
-                    for key in patch.keys():
-                        patch_info[key] = patch[key]
-                    if 'name' not in patch_info.keys():
+                    for key, cur_patch in patch.items():
+                        patch_info[key] = cur_patch
+                    if 'name' not in patch_info:
                         raise EasyBuildError("Wrong patch spec '%s', when using a dict 'name' entry must be supplied",
                                              str(patch))
                     patch = patch_info['name']

test/framework/options.py

@@ -4599,7 +4599,7 @@ def test_github_new_update_pr(self):
             '--git-working-dirs-path=%s' % git_working_dir,
             ':bzip2-1.0.6.eb',
         ])
-        error_msg = "A meaningful commit message must be specified via --pr-commit-msg"
+        error_msg = "A meaningful commit message must be specified via --pr-commit-msg.*\nDeleted: bzip2-1.0.6.eb"
 
         self.mock_stdout(True)
         self.assertErrorRegex(EasyBuildError, error_msg, self.eb_main, args, raise_error=True, testing=False)
@@ -4678,7 +4678,8 @@ def test_github_new_update_pr(self):
             gcc_ec,
             '-D',
         ]
-        error_msg = "A meaningful commit message must be specified via --pr-commit-msg"
+        error_msg = "A meaningful commit message must be specified via --pr-commit-msg.*\n"
+        error_msg += "Modified: " + os.path.basename(gcc_ec)
         self.mock_stdout(True)
         self.assertErrorRegex(EasyBuildError, error_msg, self.eb_main, args, raise_error=True)
         self.mock_stdout(False)