Add support for github organizations #1894
Conversation
An additional options build/command-line option `github-org` was added, to allow users to create pull requests from an github organization instead of a user github account. The check_github function was modified to properly check if the user has access to the repo in the github organization.
|
I didn't add tests yet, because I am not sure how to best test this. |
| # push to GitHub | ||
| github_user = build_option('github_user') | ||
| github_url = '[email protected]:%s/%s.git' % (github_user, pr_target_repo) | ||
| github_user = github_account = build_option('github_user') |
There was a problem hiding this comment.
github_user is no longer used in this function, so no need to define it?
|
@timeu it probably suffices to enhance the existing |
Functionality to retrieve github account was extracted into a helper function. Tests were modified to cover github-org paramter
| """ | ||
| if build_option('github_org'): | ||
| return build_option('github_org') | ||
| return build_option('github_user') |
There was a problem hiding this comment.
@timeu now begins the code style nitpicking... ;-)
We try to avoid having multiple return statements in functions, it makes it harder to grasp the code.
So, I suggest this (which is shorter too):
return build_option('github_org') or build_option('github_user')And now that I see it like this, maybe a function is overkill? Sorry about the back-and-forth here...
There was a problem hiding this comment.
yeah np ;)
So should I inline/remove the helper function ?
There was a problem hiding this comment.
yeah, maybe, seems a bit too silly to have a function for a simple or...
|
@timeu Almost there, just some style nitpicking, thanks for looking into this! |
|
Ok hopefully fixed all issues. I am not sure if this is due to my local development setup or maybe a bad GitPython dependency (2.08) |
|
@timeu hmm, I'm not sure why those tests fail for you, but I'm missing some info; can you please copy-paste the full output of the following command? |
|
@boegel : This is the output: |
|
@timeu Hmm, this is quite strange, I don't see why the pattern it is looking for isn't being found, since it's there... I also can't reproduce these failures on my end with your branch. This may be a locale issue, although even then I don't see what exactly. Can you try making these tests a little bit less strict by changing this: diff --git a/test/framework/options.py b/test/framework/options.py
index e945e95..2f77b09 100644
--- a/test/framework/options.py
+++ b/test/framework/options.py
@@ -2297,8 +2297,8 @@ class CommandLineOptionsTest(EnhancedTestCase):
r"^\* title: \"\{tools\}\[gompi/1.3.12\] toy v0.0\"",
r"\(created using `eb --new-pr`\)", # description
r"^\* overview of changes:",
- r".*/toy-0.0-gompi-1.3.12-test.eb\s+\|\s+[0-9]+\s+\++",
- r".*/toy-0.0_typo.patch\s+\|\s+[0-9]+\s+\++",
+ r".*/toy-0.0-gompi-1.3.12-test.eb\s*\|",
+ r".*/toy-0.0_typo.patch\s*\|",
r"^\s*2 files changed",
]
for regex in regexs:
@@ -2448,8 +2448,8 @@ class CommandLineOptionsTest(EnhancedTestCase):
regexs = [
r"^\* overview of changes:",
- r".*/foo-1\.0\.eb\s+\|\s+[0-9]+\s+\++",
- r".*/bar-2\.0\.eb\s+\|\s+[0-9]+\s+\++",
+ r".*/foo-1\.0\.eb\s*\|",
+ r".*/bar-2\.0\.eb\s*\|",
r"^\s*2 files changed",
]
|
|
Ok it works with the patch but we also need to patch the second regex block (I can add a commit for that). I think the problem is that for some reason the changeset lines contain unicode characters. |
|
@timeu ah, that would indeed explain it, and in that case I think the proposed change is the proper one If you're up for adding a commit to make that change here, please do, thanks! |
Relax regex because in some cases the changeset can contain unicode, due to colored "+".
|
Added commit to relax the regex and fix the tests |
|
lgtm, going in, thanks for your efforts @timeu! |
An additional options build/command-line option
github-orgwas added,to allow users to create pull requests from an
github organization instead of a user github account (#1890).
The check_github function was modified to properly check if the user has access
to the repo in the github organization.