Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mention linked PRs in "fully signed" comment #2148

Merged
merged 8 commits into from
Jan 17, 2024
Merged

Mention linked PRs in "fully signed" comment #2148

merged 8 commits into from
Jan 17, 2024

Conversation

iarspider
Copy link
Contributor

@smuzaffar

  1. Not sure if testing is_hold is necessary - always mention linked PRs?
  2. Maybe check if PR requires either cmsdist or cmsdata PR, not just any external one?

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @iarspider for branch master.

@iarspider, @cmsbuild, @aandvalenzuela, @smuzaffar can you please review it and eventually sign? Thanks.
@sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 16, 2024 •
edited
Loading

cms-bot internal usage

@cmsbuild
Copy link
Contributor

Pull request #2148 was updated.

@smuzaffar smuzaffar mentioned this pull request Jan 16, 2024
Closed
@cmsbuild
Copy link
Contributor

Pull request #2148 was updated.

@smuzaffar smuzaffar mentioned this pull request Jan 16, 2024
Merged
@cmsbuild
Copy link
Contributor

Pull request #2148 was updated.

smuzaffar
smuzaffar reviewed Jan 16, 2024
View reviewed changes
process_pr.py Outdated
r = gh.get_repository(linked_pr_repo)
linked_pr = r.get_issue(linked_pr_id)
if linked_pr.state != "closed":
linked_pr.create_comment(
Copy link
Contributor

Choose a reason for hiding this comment

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

@iarspider , only call to create_comment should be under not dryRun condition

smuzaffar
smuzaffar reviewed Jan 16, 2024
View reviewed changes
process_pr.py Outdated
+ ", and should probably be merged together with it"
)
messageFullySigned += (", " if prepend_comma else "") + linked_pr
prepend_comma = True
Copy link
Contributor

@smuzaffar smuzaffar Jan 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@iarspider , what if all the other PRs are merged/closed? Shouldn' we first check if there are any PRs which are still only and then add the **Notice** This .... in to messageFullySigned ?

@smuzaffar
Copy link
Contributor

@iarspider , I tried this code for cms-sw/cmssw#43719 and get this error

Traceback (most recent call last):
  File "/tmp/cmsbuild/jenkins/workspace/cms-bot/cms-bot/process-pull-request.py", line 83, in <module>
    process_pr(repo_config, gh, repo, repo.get_issue(prId), opts.dryRun, force=opts.force)
  File "/tmp/cmsbuild/jenkins/workspace/cms-bot/cms-bot/process_pr.py", line 2058, in process_pr
    linked_prs = global_test_params["PULL_REQUEST"].split()
KeyError: 'PULL_REQUEST'

@cmsbuild
Copy link
Contributor

Pull request #2148 was updated.

@iarspider
Copy link
Contributor Author

Latest changes (7886d90) not tested yet...

@cmsbuild
Copy link
Contributor

Pull request #2148 was updated.

@iarspider iarspider requested a review from smuzaffar January 17, 2024 14:42
@cmsbuild
Copy link
Contributor

Pull request #2148 was updated.

@cmsbuild
Copy link
Contributor

REMINDER @rappoccio, @antoniovilela, @sextonkennedy: This PR was tested with cms-sw/cmssw#43719, please check if they should be merged together

@smuzaffar
Copy link
Contributor

looks good

@cms-sw/orp-l2 , once this is merged then bot will add extra message like

Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: cms-repo#pr_num

e.g. see cms-sw/cmssw#43719 (comment) . Hopefully this will help you merging external PRs (e.g. cms-data PRs)

@smuzaffar smuzaffar merged commit 2d626d3 into master Jan 17, 2024
3 of 4 checks passed
@iarspider iarspider deleted the linked-prs branch January 18, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smuzaffar smuzaffar smuzaffar approved these changes

Assignees
No one assigned
Labels
externals-pending orp-pending pending-signatures tests-pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@iarspider @cmsbuild @smuzaffar