staabm / phpstan-todo-by

Todo comments with expiration
https://staabm.github.io/2023/12/17/phpstan-todo-by-published.html
MIT License
173 stars 4 forks source link

Idea: todo expire todo when PR is released #49

Closed nikophil closed 8 months ago

nikophil commented 8 months ago

Hi!

not sure you'll like this one, as it is a logic suite of https://github.com/staabm/phpstan-todo-by/issues/31, but going a little bit further :sweat_smile:

Something I'd find really useful would be to expire todo comments based on a vendor PR link: when the PR is merged and released, and vendor meets the release version in your dependencies.

I've seen a lot of projects cluttered with comments like this:

// todo: remove this workaround when https://github/vendor/lib/pull/1234 is released

and these todos are only fixed if by chance someone some day sees the comment and gives it some attention.

GitHub exposes in its api the first release when a PR appears. So, I think it could leverage the work done in #32

WDYT?

staabm commented 8 months ago

We could implement a todo-by-ticket github-issues fetcher todo that.

This should work for github issues and PRs - these are very similar on github

Just enhance the rule implemented with https://github.com/staabm/phpstan-todo-by/pull/26 with a new fetcher

Since github issue identifiers have a different pattern then jira issue keys, we might need a separate rule

nikophil commented 8 months ago

sadly, what I was thinking seems to be not doable easily: the information of the first git tag which includes the PR, which you can view in GitHub interface, is not exposed in the API.

I can't find any easy way to guess this information. The best which can be done is to expire todo comment if the issue is resolved or the PR is merged, but IMO it's less useful than the logic based on versions

staabm commented 8 months ago

the refined-github browser extensions renders a note in PRs which have been merged and are part of a PR:

grafik

https://github.com/refined-github/refined-github/blob/a101ac6513d71173d4c5788b999129d812823046/source/features/closing-remarks.tsx#L78-L91

you might find some inspiration there

nikophil commented 8 months ago

haha I didn't even know this information was added by this addon, I use it since too much time 😅

https://github.com/refined-github/refined-github/blob/a101ac6513d71173d4c5788b999129d812823046/source/features/closing-remarks.tsx#L21-L31

they look into this url and hack into the html https://github.com/staabm/phpstan-todo-by/branch_commits/98d37d9dcfbcf1a70bb7a36ecea7c6ca7430c347

it seems there is no API equivalent. This solution is a bit hacky: I think there is no BC policy on this type of html url. That why from time to time the add-on has bugs: when GH changes its urls / or html markdown. But if you're still okay.... why not ;)

staabm commented 8 months ago

Hmm I would prefer a more api centric approach

staabm commented 8 months ago

issue/pr closed state handling was implemented with https://github.com/staabm/phpstan-todo-by/pull/62

as you pointed out the 'released' state is not easily to determine. until github.com provides a proper mean to detect this state I will close this issue for now.

thanks for the feedback though

github-actions[bot] commented 7 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.