Skip to content

get pr_title and pr_descr built_options in new_pr_from_branch instead of new_pr (and commit_msg in both)#3298

Merged
boegel merged 2 commits intoeasybuilders:developfrom
migueldiascosta:fix_new_pr_from_branch_title
May 1, 2020
Merged

get pr_title and pr_descr built_options in new_pr_from_branch instead of new_pr (and commit_msg in both)#3298
boegel merged 2 commits intoeasybuilders:developfrom
migueldiascosta:fix_new_pr_from_branch_title

Conversation

@migueldiascosta
Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta commented Apr 23, 2020

fix for #3295

akesandgren
akesandgren previously approved these changes May 1, 2020
Copy link
Copy Markdown
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@akesandgren
Copy link
Copy Markdown
Contributor

@migueldiascosta Did you test it?

@migueldiascosta
Copy link
Copy Markdown
Member Author

@ake I did

I can also modify test_new_pr_from_branch in a way that fails without this PR and passes with it, e.g.

--- a/test/framework/options.py
+++ b/test/framework/options.py
@@ -3308,6 +3308,7 @@ class CommandLineOptionsTest(EnhancedTestCase):
             '--new-pr-from-branch=%s' % test_branch,
             '--github-user=%s' % GITHUB_TEST_ACCOUNT,  # used to get GitHub token
             '--github-org=boegel',  # used to determine account to grab branch from
+            '--pr-descr="an easyconfig for toy"',
             '-D',
         ]
         txt, _ = self._run_mock_eb(args, do_build=True, raise_error=True, testing=False)
@@ -3326,6 +3327,7 @@ class CommandLineOptionsTest(EnhancedTestCase):
             r"\* target: easybuilders/easybuild-easyconfigs:develop$",
             r"^\* from: boegel/easybuild-easyconfigs:test_new_pr_from_branch_DO_NOT_REMOVE$",
             r'^\* title: "\{tools\}\[system/system\] toy v0\.0"$',
+            r'^"an easyconfig for toy"$',
             r"^ 1 file changed, 32 insertions\(\+\)$",
             r"^\* overview of changes:\n  easybuild/easyconfigs/t/toy/toy-0\.0\.eb | 32",
         ]

(?)

@akesandgren
Copy link
Copy Markdown
Contributor

Then I think you should add that to the test just to prove it works :-)

@boegel boegel changed the title get pr_title and pr_descr built_options in new_pr_from_branch... get pr_title and pr_descr built_options in new_pr_from_branch instead of new_pr (and commit_msg in both) May 1, 2020
Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants