Skip to content

[TASK] Mark OutputFormat as not extendable #1131

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 4 commits into from
Mar 10, 2025
Merged

Conversation

oliverklee
Copy link
Collaborator

Also mark the constructor as @internal.

@coveralls
Copy link

coveralls commented Mar 9, 2025

Coverage Status

coverage: 55.567%. remained the same
when pulling 1368c3f on task/non-extendable-of
into 6801d26 on main.

Copy link
Collaborator

@JakeQZ JakeQZ left a comment

Choose a reason for hiding this comment

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

There's a missing full-stop in the @internal notice.

Also, I think we can be clearer in the class DocBlock that it's going to be made final, with extensibility being deprecated.

CHANGELOG.md Outdated
Comment on lines 24 to 25
- Mark `OutputFormat` as not extendable and the constructor as `@internal`
(#1131)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If my suggestion for the class DocBlock is accepted, this probably warrants a second changelog entry in the 'Deprecated' section. (But I don't think two separate PRs are needed!)

@JakeQZ
Copy link
Collaborator

JakeQZ commented Mar 9, 2025

Also, I think these changes could be made directly on the 8.x branch only, because on main they will be immediately removed.

oliverklee added a commit that referenced this pull request Mar 10, 2025
oliverklee added a commit that referenced this pull request Mar 10, 2025
@oliverklee oliverklee force-pushed the task/non-extendable-of branch from 3430390 to 1368c3f Compare March 10, 2025 08:39
@oliverklee oliverklee requested a review from JakeQZ March 10, 2025 08:40
@oliverklee
Copy link
Collaborator Author

I have just created #1133 for the backport.

As far as having this in main as well, I'd be fine either way (i.e., have it in main and v8.x or only v8.x).

JakeQZ pushed a commit that referenced this pull request Mar 10, 2025
@JakeQZ JakeQZ merged commit 9d2fd8e into main Mar 10, 2025
21 checks passed
@JakeQZ JakeQZ deleted the task/non-extendable-of branch March 10, 2025 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants