Skip to content

Add PR Links as List Items to Issues #519

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

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Conversation

saadmk11
Copy link
Contributor

Follow up of my conversation with @ezio-melotti.

Looks like this is not working for multiple PRs (python/cpython#96192). After some investigation I found that tasklists doesn't just render the new UI, it also re-writes the Issue body.

Example:

<!-- gh-linked-prs -->

https://github.com/python/cpython/issues/96192#tasklist-block-8fd60492-131e-4b4a-abab-06800aa4bb11

<!-- /gh-linked-prs -->


<!-- gh-linked-prs -->

https://github.com/python/cpython/issues/96192#tasklist-block-ea9583fe-18a8-47ee-aec9-08a62427146b

<!-- /gh-linked-prs -->

I found this after I inspected the API response for python/cpython#96192 (https://api.github.com/repos/python/cpython/issues/96192). This did not happen for the repositories I tested this PR on because it did not have this feature enabled.

This re-write causes multiple issues for us:

  1. Important As you can see from the API response above the re-write even removes the reference to the linked PR (gh-{pr-number}). This causes the bot to add the same PR multiple times to the same issue (os.path.ismount() doesn't properly use byte-paths from an os.DirEntry cpython#96192) and we have no way to check what PR was added before.
  2. The RegEx does not match and we can not be sure if the re-write behavior will remain after this feature out of beta. (All the other type of rendering features does not re-write issue body)

I would suggest switching to checklists at least for now until tasklists are out of beta. As I do not have access to any repository with this feature enabled I was not able to investigate any further.

Originally posted by @saadmk11 in #518 (comment)

In that case, let's just switch to checklists for now.

Seems like auto marking a checklist items as completed (e.g: - [x] gh-170) when a PR is merged does not work, It only works if the checklist item is a GitHub Issues. @ezio-melotti Should we still proceed with this?

Originally posted by @saadmk11 in #518 (comment)

Seems like auto marking a checklist items as completed (e.g: - [x] gh-170) when a PR is merged does not work

If that's the case (i.e. merging a PR doesn't mark it as completed in the checklist), then there is no advantage in using checklists over plain lists. The difference between checklists and plain lists is that the checklist shows the number of "tasks" at the top of the issue (as (1 of 3 tasks)) and that the PR will show up as Tracked in #XXX, however the former is useless if merged PRs are not marked as completed, and the latter is already covered by bedevere adding the issue link to each PR.

IOW, it's probably better to go back to plain lists for the time being.

Originally posted by @ezio-melotti in #518 (comment)

Related:

@codecov
Copy link

codecov bot commented Nov 16, 2022

Codecov Report

Merging #519 (b9b3a66) into main (4abd68d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #519   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         2034      2034           
  Branches       231       231           
=========================================
  Hits          2034      2034           
Flag Coverage Δ
Python_3.10.8 100.00% <ø> (ø)
Python_3.11.0 100.00% <ø> (ø)
Python_3.8.14 100.00% <ø> (ø)
Python_3.9.15 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
bedevere/util.py 100.00% <ø> (ø)
tests/test_gh_issue.py 100.00% <ø> (ø)
tests/test_util.py 100.00% <ø> (ø)

@ezio-melotti ezio-melotti merged commit ea0353f into python:main Nov 16, 2022
@saadmk11 saadmk11 deleted the use-lists branch November 16, 2022 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants