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.

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

Update qiskit_bot regex for rust file change trigger #7929

Merged
merged 5 commits into from
Apr 14, 2022

Conversation

mtreinish
Copy link
Member

Summary

This commit updates the regex path trigger used for qiskit-bot's
reviewer notification for changes to rust source files. The intent of
the regex was to match any path ending with .rs which indicates it was a
rust source file. However, the regex was overly broad and would match
any file that had rs in it as was recently found in #7924. [1] This
commit fixes this by making the regex more specific, it also adds an or
condition to trigger on changes to Cargo files in the root of the repo
which are the build configs for the rust code.

Details and comments

[1] #7924 (comment)

This commit updates the regex path trigger used for qiskit-bot's
reviewer notification for changes to rust source files. The intent of
the regex was to match any path ending with .rs which indicates it was a
rust source file. However, the regex was overly broad and would match
any file that had rs in it as was recently found in Qiskit#7924. [1] This
commit fixes this by making the regex more specific, it also adds an or
condition to trigger on changes to Cargo files in the root of the repo
which are the build configs for the rust code.

[1] Qiskit#7924 (comment)
@mtreinish mtreinish added the Changelog: None Do not include in changelog label Apr 12, 2022
@mtreinish mtreinish requested a review from a team as a code owner April 12, 2022 22:37
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Apr 12, 2022

Pull Request Test Coverage Report for Build 2168312573

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.943%

Totals Coverage Status
Change from base Build 2168310404: 0.0%
Covered Lines: 54211
Relevant Lines: 64581

💛 - Coveralls

@enavarro51
Copy link
Contributor

@mtreinish Can you add me to the list for non-pulse visualizations? I believe "(?!.*pulse.*)\bvisualization\b" will do for the regex. Thanks.

In code review enavarro51 asked to be added to the notification list
for changes to the visualization (excluding the pulse drawers). This
commit adds a new notification regex key to do this and will notify him
on PRs that touch these files.
@mtreinish
Copy link
Member Author

@mtreinish Can you add me to the list for non-pulse visualizations? I believe "(?!.*pulse.*)\bvisualization\b" will do for the regex. Thanks.

Sure, np. Done in 31b481d

qiskit_bot.yaml Outdated Show resolved Hide resolved
@mergify mergify bot merged commit 36a3d96 into Qiskit:main Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants