Skip to content

Rename LoopDestroyed to LoopExiting#2975

Merged
kchibisov merged 1 commit intomasterfrom
rib/pr/rename-loop-destroyed-loop-exiting
Jul 28, 2023
Merged

Rename LoopDestroyed to LoopExiting#2975
kchibisov merged 1 commit intomasterfrom
rib/pr/rename-loop-destroyed-loop-exiting

Conversation

@rib
Copy link
Copy Markdown
Contributor

@rib rib commented Jul 25, 2023

Considering the possibility of re-running an event loop via run_ondemand then it's more correct to say that the loop is about to exit without assuming it's going to be destroyed.

This is one of the follow ups for #2900, assuming that #2767 hopefully lands soon.

Note: the base for this PR isn't currently set to master, since it's stacked on the changes for #2767

- **Breaking** Removed `EventLoopExtRunReturn` / `run_return` in favor of `EventLoopExtPumpEvents` / `pump_events` and `EventLoopExtRunOnDemand` / `run_ondemand` ([#2767](https://github.com/rust-windowing/winit/pull/2767))
- `RedrawRequested` is no longer guaranteed to be emitted after `MainEventsCleared`, it is now platform-specific when the event is emitted after being requested via `redraw_request()`.
- On Windows, `RedrawRequested` is now driven by `WM_PAINT` messages which are requested via `redraw_request()`
- **Breaking** `LoopDestroyed` renamed to `LoopExiting` ([#2900](https://github.com/rust-windowing/winit/issues/2900))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In general we don't link to the PR changing it things.

Would suggest to drop the links, they are redundant and easily obtainable with git blame.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

surely there's no harm in making more context / information easily accessible from the changelog?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's just overly verbose for what it is, the context is already there with blame.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's not verbose when it's rendered though.

I really appreciate when I see project changelogs that link changes back to issues / PRs and find that useful.

Tracking down context with git blame can be a faff, and inevitably at some point something will be re-formatted and it becomes harder to track context down via blame.

@rib rib force-pushed the rib/pr/rename-loop-destroyed-loop-exiting branch from 615c359 to df8eaea Compare July 26, 2023 10:24
@rib rib force-pushed the rib/base/run-ondemand-and-event-pumping branch from ff4f2d4 to aa2cb60 Compare July 26, 2023 10:24
@rib rib mentioned this pull request Jul 28, 2023
17 tasks
Copy link
Copy Markdown
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Could you target a different branch?

@rib rib force-pushed the rib/pr/rename-loop-destroyed-loop-exiting branch from df8eaea to 4ed3e31 Compare July 28, 2023 13:46
@rib rib changed the base branch from rib/base/run-ondemand-and-event-pumping to master July 28, 2023 13:46
@rib
Copy link
Copy Markdown
Contributor Author

rib commented Jul 28, 2023

okey, just updated the base branch to master

The other branch that's stacked on this should automatically update it's base branch if this gets merged is just that this PR was stacked on a branch in my personal winit fork and so I had to make a dummy branch on origin that I could use as a temporary base.

@kchibisov
Copy link
Copy Markdown
Member

@rib could you rebase + force push this branch again? I'm not sure I can get CI unstuck.

Considering the possibility of re-running an event loop via run_ondemand
then it's more correct to say that the loop is about to exit without
assuming it's going to be destroyed.
@kchibisov kchibisov force-pushed the rib/pr/rename-loop-destroyed-loop-exiting branch from 4ed3e31 to aabc349 Compare July 28, 2023 15:56
@kchibisov
Copy link
Copy Markdown
Member

I've repushed to trigger CI.

@kchibisov kchibisov merged commit 935146d into master Jul 28, 2023
@kchibisov kchibisov deleted the rib/pr/rename-loop-destroyed-loop-exiting branch July 28, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants