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

Add workflow for closing issues on develop merge #1745

Merged
merged 7 commits into from
May 3, 2024

Conversation

Kr0nox
Copy link
Member

@Kr0nox Kr0nox commented May 2, 2024

This PR adds a workflow that closes issues when they get merged into the develop branch.

The workflow itself starts a python script for this.
This script uses the GitHub GraphQL API to get all issues that were linked using the sidebar. It then parses the text of the PR to get all issues that are linked using closing keywords.
It then uses the REST API to close the linked issues.

As an example we have linked this issue:
closes #1597

@Kr0nox Kr0nox added the github_actions Pull requests that update GitHub Actions code label May 2, 2024
@Kr0nox Kr0nox requested a review from a team May 2, 2024 12:48
@tsaglam
Copy link
Member

tsaglam commented May 2, 2024

@dfuchss this is the PR I mentioned.

Copy link
Member

@dfuchss dfuchss left a comment

Choose a reason for hiding this comment

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

Sounds good. Just two little improvement suggestions and then it should be ready to merge from my point of view. Nevertheless, I think it has to be merged to main too, that it works.

.github/workflows/closeDevIssues.yml Outdated Show resolved Hide resolved
.github/workflows/scripts/closeDevIssues.py Outdated Show resolved Hide resolved
.github/workflows/scripts/closeDevIssues.py Show resolved Hide resolved
@Kr0nox
Copy link
Member Author

Kr0nox commented May 2, 2024

Nevertheless, I think it has to be merged to main too, that it works.

In my testing it always worked when I did not have the workflow on main and also always took the newest version of the workflow, that was not on main. This would mean it takes the workflow from either the target branch or the branch the PR comes from

Kr0nox and others added 2 commits May 2, 2024 17:30
Co-authored-by: Timur Sağlam <timur.saglam@kit.edu>
@dfuchss
Copy link
Member

dfuchss commented May 2, 2024

Nevertheless, I think it has to be merged to main too, that it works.

In my testing it always worked when I did not have the workflow on main and also always took the newest version of the workflow, that was not on main. This would mean it takes the workflow from either the target branch or the branch the PR comes from

Ok. Then it might not be needed.

@Kr0nox Kr0nox requested a review from dfuchss May 2, 2024 18:48
@Kr0nox
Copy link
Member Author

Kr0nox commented May 3, 2024

Now that we have the opportunity to close the issues with a custom message, do we want to add that the feature or bug fix will be released with the next release, so users do not think the feature is there already.
Currently, the bot just says Closed by #NUMBER. We could have them say something like:

Closed by #NUMBEr
The fix/feature will be part of the next release. If you want it now, you can build JPlag from the sources of the develop branch.

What are your opinions on that?

@tsaglam
Copy link
Member

tsaglam commented May 3, 2024

I think I would this is probably not needed, as the users who actually follow the development here are aware that there is a delay between the PR merge and the feature release. I would rather just stick to Closed by #NUMBER.

@Kr0nox
Copy link
Member Author

Kr0nox commented May 3, 2024

the users who actually follow the development here

My though was that users that post issues might not do that and expect closed issues to be solved and ready for use, but in the end its up to you

Co-authored-by: Timur Sağlam <timur.saglam@kit.edu>
@tsaglam
Copy link
Member

tsaglam commented May 3, 2024

At least from my experience, we have not yet received questions regarding such a case. We can always add it if this appears more frequently than in the past.

@Kr0nox
Copy link
Member Author

Kr0nox commented May 3, 2024

ok, then from my point of view this should be ready to merge

@tsaglam tsaglam added enhancement Issue/PR that involves features, improvements and other changes minor Minor issue/feature/contribution/change labels May 3, 2024
@tsaglam tsaglam merged commit 8afd33f into develop May 3, 2024
@tsaglam tsaglam deleted the dev-issue-close-workflow branch May 3, 2024 07:45
@tsaglam
Copy link
Member

tsaglam commented May 3, 2024

@Kr0nox The workflow affected more PRs than expected, looks like a bug? (see referenced issues above)

EDIT: This is a matching issue, you are matching:

  • 1
  • 15
  • 159
  • 1597

@dfuchss
Copy link
Member

dfuchss commented May 3, 2024

Yes 1 .. 15 .. 159 .. 1597 .. there is probably a \b or similar missing :) Even the first issue is effected

@Kr0nox
Copy link
Member Author

Kr0nox commented May 3, 2024

Yes, we did not check for a non number to be after the issue number, so it found all substrings of numbers of the actual issue.
#1748 provides a fix for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR that involves features, improvements and other changes github_actions Pull requests that update GitHub Actions code minor Minor issue/feature/contribution/change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants