Skip to content

fix target account for --update-pr in case it's different from GitHub account being used to push branch#2419

Merged
wpoely86 merged 5 commits intoeasybuilders:3.5.xfrom
boegel:fix_update_pr_target_account
Mar 2, 2018
Merged

fix target account for --update-pr in case it's different from GitHub account being used to push branch#2419
wpoely86 merged 5 commits intoeasybuilders:3.5.xfrom
boegel:fix_update_pr_target_account

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Mar 1, 2018

Sometimes --github-user is not the same as the account/organisation that was used to open a PR, cfr. bug report by @shahzebsiddiqui in #2418...

Given it's limited scope & impact I consider this a candidate for a last-minute inclusion in EasyBuild v3.5.2...

@boegel boegel added the bug fix label Mar 1, 2018
@boegel boegel added this to the 3.5.2 milestone Mar 1, 2018
@boegel boegel requested a review from wpoely86 March 1, 2018 22:00
@boegel
Copy link
Copy Markdown
Member Author

boegel commented Mar 2, 2018

tests fail because of SourceForge downtime, I propose we work around this by temporarily disabling those tests, cfr. #2420

@easybuilders easybuilders deleted a comment from boegelbot Mar 2, 2018
Comment thread easybuild/tools/github.py Outdated

if not dry_run:
_log.debug("Pushing branch '%s' to remote '%s' (%s)", pr_branch, remote_name, github_url)
_log.debug(push_log_msg)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

where is the definied? Don't you mean push_branch_mg?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ugh, nice catch, thanks.

I fixed this by logging via print_msg (which is covered by the unit tests). The non-dry-run --update-pr is not covered by the tests currently (maybe we should look into mocking the actual push rather than only checking the dry-run...)

boegel added a commit to boegel/easybuild-framework that referenced this pull request Mar 2, 2018
Comment thread easybuild/tools/github.py Outdated

if not dry_run:
_log.debug(push_log_msg)
print_msg(push_branch_msg + '[DRY RUN]', log=_log)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

space between push_branch_msg and [DRY RUN] ?

@wpoely86 wpoely86 merged commit 7426a9b into easybuilders:3.5.x Mar 2, 2018
@boegel boegel deleted the fix_update_pr_target_account branch March 2, 2018 10:28
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.

2 participants