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

Finalize issues #7595

Merged
merged 2 commits into from
Feb 6, 2022
Merged

Finalize issues #7595

merged 2 commits into from
Feb 6, 2022

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Feb 5, 2022

No description provided.

@weirdan weirdan added the release:removed The PR will be included in 'Removed' section of the release notes label Feb 5, 2022
@weirdan weirdan added this to the Psalm 5 milestone Feb 5, 2022
@weirdan weirdan requested a review from orklah February 5, 2022 22:34
@weirdan weirdan merged commit 7b8dc99 into vimeo:master Feb 6, 2022
@weirdan weirdan deleted the finalize-issues branch February 6, 2022 20:53
@pilif
Copy link
Contributor

pilif commented Feb 6, 2022

This is indeed a bit of a shame.

In our code-base, we have some legacy patterns we want to discourage and I wrote a plug-in to find those and raise a deprecation issue.

But I’m reporting an anonymous subclass of the issue and set the URL to point to our wiki with a more detailed explanation (by overriding toIssueData)

If you need to make these final, please consider letting plugins provide URLs and message overrides.

@weirdan
Copy link
Collaborator Author

weirdan commented Feb 6, 2022

Is there any reason you're not extending PluginIssue instead?

abstract class PluginIssue extends CodeIssue

@pilif
Copy link
Contributor

pilif commented Feb 7, 2022

Because the pattern is about a deprecation, so using the native deprecation made sense with regards to configuration how to deal with deprecations in which directories.

Basing the issue in PluginIssue produces a secondary hierarchy of issues which needs a separate configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:removed The PR will be included in 'Removed' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants