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

Release 0.27.0 version #2322

Merged
merged 1 commit into from
Jul 26, 2022
Merged

Conversation

kchibisov
Copy link
Member

@kchibisov kchibisov commented Jun 10, 2022

I think it's time to cut a new release.

If there're suggestion on what should made into it let me know.


Blocked on:

Post release checklist:

@kchibisov
Copy link
Member Author

@msiglreith @MarijnS95 there're notes in Cargo.toml wrt syncing releases with android stuff. So I think that's time to resolve it.

@MarijnS95
Copy link
Member

That note links to #1995, of which some tasks are still outstanding.

@kchibisov What's your timeline for this? There are some larger Android issues I'd have liked to flesh out first, primarily around unifying how the Suspended/Resumed events work across platforms (CC @rib) but that's not something we can fix overnight, this 0.27 bump comes a bit sudden 😅

Afaik/typically the winit maintainers start out with a tracking issue, listing what PRs should make it in and what issues should be resolved before merging, similar to a GitHub milestone. Is that something we'd like to do here? For starters I'd add this PR to that list: #2307.

@kchibisov
Copy link
Member Author

Afaik/typically the winit maintainers start out with a tracking issue, listing what PRs should make it in and what issues should be resolved before merging, similar to a GitHub milestone. Is that something we'd like to do here? For starters I'd add this PR to that list: #2307.

There's a milestone for the 0.27 version, not sure the separate issue is required and before we were releasing without those.

You can explicitly block this PR by linking to other PRs, so we'll have everything in place.

@MarijnS95
Copy link
Member

Perfect, I hadn't seen the milestone yet but have now inserted some items that I think we should at least look at. We can always collectively decide to postpone them to 0.28, but I'd like that to be an explicit, thoughtful decision :)

@MarijnS95 MarijnS95 added this to the Version 0.27 milestone Jun 10, 2022
@MarijnS95
Copy link
Member

It doesn't seem like we can have cross-repo issues in this milestone. Perhaps interesting to try (for the next release?) beta project boards, it can have issues across repos (and across orga's even, so any public/private GH repo). I've added android-ndk-rs issues to the "sync ndk release" issue for now, but can also try out the board if you like.

(Side-note: I'm working on fixing the CI issue 😉 for Android)

@madsmtm
Copy link
Member

madsmtm commented Jun 10, 2022

There has been discussion about a new release for a while now: #2234

I'm fine with waiting a bit, but I do agree that it would make sense to release 0.27.0 soon-ish, it contains both the windows-sys change and the new IME setup, and the changelog entry is getting larger than usual - so maybe it makes sense to release winit 0.27.0 with ndk 0.6.0 for now? (Apologies, I should have thought of that in #2318).

Note that there is nothing that says we can't release 0.28.0 fairly soon afterwards, depending on when your Android changes are done!


I dislike the whole GitHub projects stuff a lot (actually dislike essentially all of the newer GitHub features, probably getting old!), the tracking issue you have in #2307 is in my opinion a much easier way of doing this!

CHANGELOG.md Outdated
@@ -6,7 +6,7 @@ Please keep one empty line before and after all headers. (This is required for `

And please only add new entries to the top of this list, right below the `# Unreleased` header.

# Unreleased
# 0.27.0
Copy link
Member

@madsmtm madsmtm Jun 10, 2022

Choose a reason for hiding this comment

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

Please keep the # Unreleased header - and there should be a date here as well (when we know the actual release date)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that this date makes any sense here. The date would be on github release anyway and on the tag. Having date here doesn't provide any value at all. And if someone really want that date they can check blame as well. it's just nearly impossible to keep this date right, also it's not accurate due to timezones, etc, etc.

Removing unreleased is also expected and widely used.

Copy link
Member

Choose a reason for hiding this comment

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

Removing unreleased is also expected and widely used.

That only seems annoying / cause extra conflicts for the next contributor?

Copy link
Member

@maroider maroider Jun 10, 2022

Choose a reason for hiding this comment

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

I think it's fine to re-add the # Unreleased header in the next PR to be merged. It's not a difficult change, and we're usually given permission to push to the PR author's branch, so we're not necessarily going to have to wait on the PR author to resolve that.

As for the date: I don't think it's a problem that it can be off by a day or so. Maybe having a placeholder like 2022-XX-XX before it's clear when the update will happen could be "good enough". I do have to agree that it doesn't add much immediate value, but I consider it a "nice to have" sort of thing.

Copy link
Member

Choose a reason for hiding this comment

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

we're usually given permission to push to the PR author's branch, so we're not necessarily going to have to wait on the PR author to resolve that.

Right, this was more of a problem when we didn't do that.

Re dates: I meant this review most to remember to actually include the date. I think it's nice to have dates in changelogs, but will gladly discuss in another issue if we should stop doing so (though I think we should remove them from the rest of the changelog as well if so).

@MarijnS95
Copy link
Member

There has been discussion about a new release for a while now: #2234

Not much discussion, just a request to finally have windows-rs (from my team 👋 - we're using a git reference for some time to test it all out, so no rush from our end).

I'm fine with waiting a bit, but I do agree that it would make sense to release 0.27.0 soon-ish

Understood, I'd also like to get this out semi-soon but I hope we have a couple days to smoothen up the last rough edges. If I had known this earlier, I'd have started finalizing earlier (but then some comments and issues only came up yesterday, working on those now).

so maybe it makes sense to release winit 0.27.0 with ndk 0.6.0 for now? (Apologies, I should have thought of that in #2318).

Nah, I think we can get it over fairly soon. I would like peoples thoughts on the Android-only situation around Suspended and Resumed though!

Note that there is nothing that says we can't release 0.28.0 fairly soon afterwards, depending on when your Android changes are done!

I've previously had a bunch of disgruntled users after making many breaking releases in short succession, because the whole ecosystem has to upgrade - even if the breakage didn't affect anything major (i.e. just a niche API change that's typically breaking semver-wise).

I dislike the whole GitHub projects stuff a lot (actually dislike essentially all of the newer GitHub features, probably getting old!), the tracking issue you have in #2307 is in my opinion a much easier way of doing this!

It's slow but there hasn't been anything better for me to manage cross-organization TODOs!

@MarijnS95
Copy link
Member

MarijnS95 commented Jun 10, 2022

Reminder: the ndk (not the ndk-build+cargo-apk tooling) crates have accumulated quite some breaking changes too, and releasing together with a breaking winit release seems the best course of action. That way the ecosystem can bump both at once since they're so tightly integrated.

@kchibisov Should we have a preliminary bump in the README compatibility table, in case we forget when actually releasing + bumping those ndk crates?

@maroider maroider linked an issue Jun 10, 2022 that may be closed by this pull request
@madsmtm
Copy link
Member

madsmtm commented Jun 10, 2022

Not much discussion

True, didn't really mean the word "discussion" more "people have wanted a release for a while". Mostly wanted to link to the issue.

Nah, I think we can get it over fairly soon

Cool, I'm fine with holding off for a month or something.

I would like peoples thoughts on the Android-only situation around Suspended and Resumed

Yeah, will take a look at it at some point, just a lot of text to read through!

@MarijnS95
Copy link
Member

MarijnS95 commented Jun 10, 2022

True, didn't really mean the word "discussion" more "people have wanted a release for a while". Mostly wanted to link to the issue.

No worries, the next release is a little overdue given how many improvements there are, so better get it over with quickly especially if it has been requested multiple times.

Cool, I'm fine with holding off for a month or something.

Oh I wouldn't have wanted to hold this much longer than a week, pending linked PRs in the tracking issue. That ought to be enough time to at least get some discussion going on the outstanding Android issues (that don't even have PR proposals yet) 🤞

Yeah, will take a look at it at some point, just a lot of text to read through!

Yes indeed, information is abundant and replicated across multiple PRs and issues while lacking focus; we should perhaps make smaller tracking issues for each of the subtopics and separately schedule+tackle them.

@kchibisov
Copy link
Member Author

@kchibisov Should we have a preliminary bump in the README compatibility table, in case we forget when actually releasing + bumping those ndk crates?

Yeah, that's fine.

@kchibisov
Copy link
Member Author

Are there any news wrt android backend? Since it's getting some time to release winit. I'm not in a rush, I just won't this to stall for months.

Though, we do have some macOS stuff pending.

@MarijnS95
Copy link
Member

@kchibisov I gave up my entire weekend ±2.5 weeks ago to address all the lingering issues and breaking changes that were yet to be done, as well as a mountain of followup problems discovered along the way.

Some PRs have been approved and merged but most are waiting for review: https://github.com/rust-windowing/android-ndk-rs/pulls

I have a few more changes to make, but am waiting for review and merge to not get literally lost in branches and inter-PR dependencies.

CC @dvc94ch @msiglreith, would you be willing to pick up this mountain of work?

@MarijnS95 MarijnS95 closed this Jun 29, 2022
@MarijnS95 MarijnS95 reopened this Jun 29, 2022
@MarijnS95
Copy link
Member

Gah, I keep forgetting that GitHub has a bug today where ctrl+enter closes a PR, whereas it's only supposed to do that with ctrl+shift+enter.

@dvc94ch
Copy link
Member

dvc94ch commented Jun 29, 2022

Maybe make more frequent releases instead of trying to do it all at once. I'm currently busy

@kchibisov
Copy link
Member Author

The problem right now is that we're using ndk git, and I don't really want to use git versions in releases.

@MarijnS95
Copy link
Member

@kchibisov David is referring to making more releases of the ndk. For example release one right now to unblock winit, while leaving all the other fixes (some are breaking changes iirc) to a followup.

The git reference points to an ndk commit on master, I wouldnt've contributed a git reference to a PR / loose branch.

If we do more (breaking) ndk releases that implies winit should release more often as well, to stay in sync. However, I've been reprimanded in other repos for doing too many breaking releases as the whole ecosystem repeatedly had to upgrade. That's why I rather get a whole bunch of breaking changes in at once, and limit ourselves to patch releases for a while after. At the same time the PRs are already open and should be relatively trivial to go through, so I don't think it's really worth postponing them. Someone without Android experience may be able to go through most of them as well, and otherwise I might as well merge them on my own (as a last resort, I'd rather have some sort of peer review).

Are there any other semi-active folks around here that can deal with Android? Perhaps @rib would be able to review some of it (ndk-glue locks and wrapper), @JasperDeSutter also still seems to be around.

@MarijnS95 MarijnS95 closed this Jun 29, 2022
@MarijnS95 MarijnS95 reopened this Jun 29, 2022
@kchibisov
Copy link
Member Author

Yeah, that will make some sense, since android is a niche platform, making more releases in winit would be also fine, I guess.

@dvc94ch
Copy link
Member

dvc94ch commented Jun 29, 2022

@MarijnS95 feel free to merge the PRs. I don't think there is anything wrong with breaking releases. We have ndk-context to prevent breaking releases from breaking apps.

@kchibisov I think winit should probably have more releases too. Maybe a quarterly release schedule would help, then android-ndk-rs can coordinate its releases to coincide.

@MarijnS95
Copy link
Member

The only thing wrong with breaking releases is forcing the ecosystem to update. Currently winit was largely blocking that, but that should be solved if it releases updates more often.

Again, ndk-context doesn't solve a desync between ndk-glue in the caller and ndk-glue in winit, since they still use the static getters (need access to the activity, event poller, input queue and window). We're bringing up the same plans to solve that.

I don't like merging my own PRs without formal review, though.

@maroider
Copy link
Member

maroider commented Jul 3, 2022

I can take a look at your PRs if you'd like, although I'm far from knowledgeable about Android, nor am I able to test your changes.

@MarijnS95
Copy link
Member

@maroider Thanks, that'd be appreciated. No need to test it, and many changes are rather "Rust-y" without really requiring much knowledge about Android at all. If you hadn't seen it yet I typically am rather descriptive and verbose in my PRs (though not always concise...) so you should be able to follow along and validate the status-quo by following links.

Fortunately @msiglreith already covered some of the load, and we got a bunch of PRs in, thanks!

@msiglreith
Copy link
Member

I'm trying to slowly catch up with open topics after my sick leave - may take a few more days until I looked at everything winit related (:

@MarijnS95
Copy link
Member

Get well soon @msiglreith! Thanks a lot for covering most of the outstanding PRs thus far! I think we only have ±3 or so to go, before I can make another NDK release and unblock this winit release :)

@maroider
Copy link
Member

maroider commented Jul 16, 2022

Welp, I guess I didn't get around to helping with those PRs, although it seems that all of them are merged now.

In other news, it seems to me like we're going to update to raw-window-handle (#2376) for v0.27.0, unless there's a really good reason not to.

@MarijnS95
Copy link
Member

Welp, I guess I didn't get around to helping with those PRs, although it seems that all of them are merged now.

No worries, we're almost there! I just opened one more PR and have yet another in the pipelines (non breaking for now, though...) then I'll cut a release.

Shall we set a deadline on this week? If it doesn't make it in, tough luck and we'll release NDK + winit without those PRs.

In other news, it seems to me like we're going to update to raw-window-handle (#2376) for v0.27.0, unless there's a really good reason not to.

I think this is good, though there's no grace period now like with the previous implementation of HasRawWindowHandle 0.3 for (Has?)RawWindowHandle 0.4 so that older crates can accept a "newer" winit handle. Users can still piece them together though, all the types and members are pub.

@kchibisov
Copy link
Member Author

This week sounds good to me. I'd like to get #2378 solved first though. Solving it should be pretty straightforward and should help downstream deal with Xlib error handling in a safer way.

@kchibisov
Copy link
Member Author

Ok, we're again at the point when only android stuff is left on the plate.

@kchibisov
Copy link
Member Author

I've updated PR and it should resolve raised issues.

@maroider
Copy link
Member

I'd like to get in #2331 if we can, but I'm not sure if we ought to block on it or not. It should admittedly be fairly trivial to merge at this point.

@maroider
Copy link
Member

maroider commented Jul 23, 2022

I'm also assuming we're still blocked on rust-mobile/ndk#314, even though it doesn't seem to be linked/mentioned in #1995.

@kchibisov
Copy link
Member Author

I'm also assuming we're still blocked on rust-mobile/ndk#314, even though it doesn't seem to be linked/mentioned in #1995.

That's correct, we can't release, unless android is released.

@MarijnS95
Copy link
Member

MarijnS95 commented Jul 23, 2022

The only breaking change I'd still like to get in is rust-mobile/ndk#320, anything planned after that is nonbreaking additive API that we can also do patch releases for. Anything else will have to be postponed for the next breaking bump - and we're in a much better position now where I don't really want to make more breaking changes anyway unless the API is clearly wrong.

@MarijnS95
Copy link
Member

Still waiting for a positive review on rust-mobile/ndk#314 after whose merge the updates will be automatically published to crates.io.

@kchibisov
Copy link
Member Author

I'll make a release right after #2397 gets resolved. So if anyone has concerns wrt that please let me know.

@kchibisov
Copy link
Member Author

The issue got resolved. Will release this evening.

@madsmtm
Copy link
Member

madsmtm commented Jul 26, 2022

Thanks a lot for pushing through on this @kchibisov!

@kchibisov kchibisov merged commit 5003564 into rust-windowing:master Jul 26, 2022
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.

Please publish a new release
6 participants