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

Removed 0.4.0 deprecations, for 0.5.0 #3059

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

HalfWhitt
Copy link
Contributor

@HalfWhitt HalfWhitt commented Dec 22, 2024

With 0.5.0 impending, this PR removes everything that was deprecated in 0.4.0 or earlier, which consists of:

  • App

    • __init__ parameters: id and windows
    • name
    • windows.setter
  • Window

    • __init__ parameters: resizeable and closeable
    • multiselect parameter to the dialog methods (which themselves are deprecated, but more recently)
    • __iadd__ and __isub__
  • Canvas

    • new_path
    • move_to
    • line_to
    • bezier_curve_to
    • quadratic_curve_to
    • arc
    • ellipse
    • rect
    • write_text
    • rotate
    • scale
    • translate
    • reset_transform
    • closed_path
    • fill
    • stroke
    • tight parameter to measure_text
  • Stroke

    • new_path
    • context
    • closed_path
    • __enter__ and __exit__
  • Context

    • preserve parameter to fill
  • Fill

    • __enter__ and __exit__
  • DetailedList

    • on_delete (property and __init__ parameter)
  • NumberInput

    • min_value and max_value (properties and __init__ parameters)
  • Selection

    • on_select (property and __init__ parameter)
  • Slider

    • range (property and __init__ parameter)
  • Table

    • on_double_click (property and __init__ parameter)
    • add_column
  • TimePicker

  • DatePicker

  • Tree

    • on_double_click (property and __init__ parameter)
  • Icon

    • TOGA_ICON
  • NSStringDrawingDisableScreenFontSubstitution

  • NSStringDrawingOneShot

Icon.TOGA_ICON is a bit of a stretch, since it wasn't actually marked as deprecated until 4.0.1. But it was already a redundant alias for DEFAULT_ICON since at least 2022.

I've also gone through and made remaining deprecation comments more consistent, adding missing dates as well as adding version info as accurately as I could manage. I opted to use the current version when each shim was merged, rather than the version in which it was released — e.g. "compatibility for <= 0.4.5" rather than "for < 0.4.6". My reasoning is that while this might not be quite as intuitive when going back and reading after the fact, it's more feasible for including in new code, since one may not know if the next release will be a version bump or not.

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@HalfWhitt
Copy link
Contributor Author

By the way, on the subject of digging through previous PRs and file states to track things down... Have you ever considered using squash-and-merge for PRs here? I noticed you did so, in fact, for the window states PR #2473 (added as a single commit, 1c8b7d4). I imagine in that case it was just because it contained 301 commits. But if all PRs were merged that way, it would make Toga's overall commit history a lot easier to follow, and the granular info would still be available on the PR.

@freakboy3742
Copy link
Member

With 0.5.0 impending, this PR removes everything that was deprecated in 0.4.0 or earlier, which consists of:
...

These are all as expected, stemming from the widget audit that we spent most of last year working on. The only two that stood out are:

  • NSStringDrawingDisableScreenFontSubstitution
  • NSStringDrawingOneShot

because they're Cocoa-specific symbols. They're marked as deprecated because they're deprecated in Cocoa, but still valid... but given they're deprecated, I guess there's no value in us carrying them.

Icon.TOGA_ICON is a bit of a stretch, since it wasn't actually marked as deprecated until 4.0.1. But it was already a redundant alias for DEFAULT_ICON since at least 2022.

I can live with this. 0.4.0 is still over a year old, and this has the added benefit that it removes the source of a docs-building warning that has been an annoying wart.

I've also gone through and made remaining deprecation comments more consistent, adding missing dates as well as adding version info as accurately as I could manage. I opted to use the current version when each shim was merged, rather than the version in which it was released — e.g. "compatibility for <= 0.4.5" rather than "for < 0.4.6". My reasoning is that while this might not be quite as intuitive when going back and reading after the fact, it's more feasible for including in new code, since one may not know if the next release will be a version bump or not.

Thanks - that's much appreciated.

By the way, on the subject of digging through previous PRs and file states to track things down... Have you ever considered using squash-and-merge for PRs here? I noticed you did so, in fact, for the window states PR #2473 (added as a single commit, 1c8b7d4). I imagine in that case it was just because it contained 301 commits. But if all PRs were merged that way, it would make Toga's overall commit history a lot easier to follow, and the granular info would still be available on the PR.

Yeah - I've thought about it from time to time. My original thinking was that keeping the commit history costs nothing, and might be useful in code archaeology - but I guess we can extract that from the PR if it's ever useful. You're correct that #2473 was merge-committed because it had a lot of history, and most of that history wasn't going to be helpful for diagnostic purposes.... but if that's true for one PR, it's likely true for others.

So - with 0.5.0 on the horizon, maybe it would be worth adopting merge commits as a repo policy change...

@rmartin16
Copy link
Member

rmartin16 commented Dec 23, 2024

So - with 0.5.0 on the horizon, maybe it would be worth adopting merge commits as a repo policy change...

+1 - trying to git blame something has been a major pain at times (especially in CI workflows where there's lots of "disabling tests" commits)

@HalfWhitt
Copy link
Contributor Author

The only two that stood out are:

  • NSStringDrawingDisableScreenFontSubstitution
  • NSStringDrawingOneShot

because they're Cocoa-specific symbols. They're marked as deprecated because they're deprecated in Cocoa, but still valid... but given they're deprecated, I guess there's no value in us carrying them.

Not only are they deprecated — if I'm reading their doc pages correctly, they haven't even been available since 10.11, which we don't support anyway.

Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

All fairly straightforward. Thanks for the cleanup!

@freakboy3742 freakboy3742 merged commit b25b4e3 into beeware:main Dec 23, 2024
41 checks passed
@HalfWhitt HalfWhitt deleted the remove_deprecations branch December 23, 2024 11:05
@mhsmith
Copy link
Member

mhsmith commented Dec 26, 2024

+1 - trying to git blame something has been a major pain at times (especially in CI workflows where there's lots of "disabling tests" commits)

I agree, and even in non-CI PRs, a lot of the commits have messages like "fix typos" or "add tests", which aren't helpful in the context of a blame. From what I've seen in the CPython repository, which uses a squash workflow, I think the positives definitely outweigh the negatives.

@HalfWhitt
Copy link
Contributor Author

I agree, and even in non-CI PRs, a lot of the commits have messages like "fix typos" or "add tests", which aren't helpful in the context of a blame.

Yeah, under normal circumstances I don't think anyone is just dying to know how many times I've saved a changenote in the wrong folder at first. 🤣

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.

4 participants