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.20.0 Alpha 1 #913

Merged
merged 1 commit into from
Jun 21, 2019
Merged

Conversation

Osspial
Copy link
Contributor

@Osspial Osspial commented Jun 13, 2019

As per #830 (comment). I want to get #911 merged and #891 resolved before this, but... yeah. We've still got a long way for Winit to go, but we've made it this far, and I can only see progress speeding up from here. I cannot express enough thanks to the people that have helped get it here, from those writing the code, to the people testing stuff out, the people contributing their thoughts in the issues, and everyone in-between.

It's kinda surreal to be able to do this.

🎉

Closes #825, closes #276, closes #794, closes #728, closes #413, closes #219, closes #459.

@ghost
Copy link

ghost commented Jun 13, 2019

I've rebased #909 if we want to get that in as well. I assume we're fine with the Android and Emscripten backends not compiling for this release?

Copy link
Contributor

@goddessfreya goddessfreya left a comment

Choose a reason for hiding this comment

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

🎉 🎉 🎉

@Osspial
Copy link
Contributor Author

Osspial commented Jun 13, 2019

@aleksijuvani Thanks! We should get that PR in, too.

I'm fine with Android not compiling, since waiting for it to get done without anyone on board that can actually do it is a recipe for waiting forever. As far as Emscripten goes... well, I'm not actually sure what we're going to end up doing with the Emscripten backend. We've got a couple people working on wasm backends, and once those get merged I can't see Emscripten getting much use since it's such a pain to set up. I'm inclined to just remove it, but there hasn't been any formal talk about that, so I'm not going to pass judgement on it just yet.

@ghost
Copy link

ghost commented Jun 13, 2019

Right, I figured that was the case! Regarding Android, technically, I think anyone could do it, seeing as how the Android simulator is freely available for download. It's just that it's hard to justify putting in the effort for a platform that the person doing the work isn't personally invested in.

@elinorbgr
Copy link
Contributor

elinorbgr commented Jun 13, 2019

Going to do a quick a PR for #891

EDIT: see #916

@elinorbgr
Copy link
Contributor

It's kinda surreal to be able to do this.

And pretty relieving as well! Glad to see we finally reached this point. 🎉

@felixrabe
Copy link
Contributor

felixrabe commented Jun 17, 2019

From my POV, you can just merge this. It's not as if you were instantly making a release, but preparing for it. (IMO, immediately after a release, Cargo.toml version and CHANGELOG header should reflect an upcoming version instead of the last released one anyway.) (Edit: Yay, I can approve stuff! :) )

@Osspial
Copy link
Contributor Author

Osspial commented Jun 18, 2019

@felixrabe The CI is set up to automatically release when the version gets incremented on master.

@elinorbgr
Copy link
Contributor

@Osspial The two issues you linked in the original post are now resolved though, are there other things you want to finish before releasing the alpha?

@Osspial
Copy link
Contributor Author

Osspial commented Jun 18, 2019

@vberger Nope, stuff looks good! I'll pull together a post for this and release this/announce the marking push at some point later today.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 18, 2019

@zegentzy is Glutin ready for this release?

@goddessfreya
Copy link
Contributor

goddessfreya commented Jun 18, 2019

It's short an iOS backend. Beyond that, I guess it's releaseable, although I didn't get in nearly the amount of things I wanted done, due to exams hoovering up my time.

EDIT: I say go for it. I don't think anyone's planing on writing an iOS backend for glutin, so I don't think waiting around will earn us anything.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 19, 2019

First draft of release post, if anyone would like to feedback it. That combines the contributor marketing push with some new stuff detailing the major changes we've made and why.

There are still placeholder links in there that I'd like to fix, but I'm holding off on doing those until I've made all the things to link to.

@elinorbgr
Copy link
Contributor

This drafts looks pretty awesome ! 👍 I personally have no changes to suggest.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 20, 2019

Alright, stuff is pretty much ready! Took longer than I expected, but hell, what doesn't?

I'll hold off on posting this until tomorrow morning so we can catch the reddit/hackernews visibility wave, but we should get the alpha onto crates.io sometime later tonight so we can be sure that everything is working as we'd like.

WRT the issue list here: it's probably best to just tag those issues and link to our issue page, with the appropriate filters. That'll be more accurate than maintaining a separate list of issues we need help on.

@goddessfreya
Copy link
Contributor

goddessfreya commented Jun 20, 2019

WRT the issue list here: it's probably best to just tag those issues and link to our issue page, with the appropriate filters. That'll be more accurate than maintaining a separate list of issues we need help on.

On it.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 20, 2019

I'm using GitHub's pinned issues feature to highlight important issues related to this release. If anyone thinks other issues are more important than what's been pinned, please raise your concerns here.

@goddessfreya
Copy link
Contributor

And done!

@Osspial
Copy link
Contributor Author

Osspial commented Jun 21, 2019

@zegentzy Thanks!

One thing - why do we want a Contributor Push Issue tag? I'm not entirely sure what it's intended to communicate.

@goddessfreya
Copy link
Contributor

"This issue looks relatively still up to date to us, and should be ready-ish for solving." The name Contributor Push Issue doesn't exactly convey that, but I couldn't think of one better.

I'm sure it's quickly going to fall out of sync, but oh well.

@aloucks
Copy link
Contributor

aloucks commented Jun 21, 2019

This might be a good time to run cargo fmt (once all outstanding 0.20-alpha PRs are merged). I'd also recommend adding cargo fmt --check to CI to enforce it.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 21, 2019

@zegentzy if that's the case, I think I'd rather just remove the tag. I'm not sure it conveys enough information to be useful, and it's unclear when to apply it - the meanings you've applied to it seem to be already covered by other tags.

@aloucks That's a pretty good idea! I'll go ahead and pull that together.

@Osspial Osspial merged commit 8d6e8bb into rust-windowing:master Jun 21, 2019
@Osspial
Copy link
Contributor Author

Osspial commented Jun 21, 2019

https://crates.io/crates/winit
https://crates.io/crates/glutin
https://users.rust-lang.org/t/winit-0-20-the-state-of-windowing-in-rust-and-a-request-for-help/29485
https://www.reddit.com/r/rust/comments/c3cmqb/winit_020_the_state_of_windowing_in_rust_and_a/
https://www.reddit.com/r/programming/comments/c3csb0/winit_020_the_state_of_windowing_in_rust_and_a/
https://news.ycombinator.com/item?id=20243853

It is done.

@aloucks
Copy link
Contributor

aloucks commented Jun 21, 2019

Nice!

One thing -- you might consider re-formatting using rustfmt with only stable options. I accidentally ran cargo fmt after pulling down the latest code and nearly every file was modified. I didn't realize what the problem was until I went spunking through this PR and found that it used +nightly. Is requiring nightly the right choice? It could cause some friction with PRs.

@ghost
Copy link

ghost commented Jun 22, 2019

If I remember correctly, I used to use rustfmt with +nightly and I found that it occasionally made significant formatting changes that broke the build when used in conjunction with --check. I would recommend switching to stable rustfmt.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 22, 2019

@aloucks @aleksijuvani I'd be fine with switching to stable rustfmt. I normally live on nightly, so I didn't notice that some of the options were unstable 😄!

That being said, if we're sticking with stable rustfmt I think we should disable formatting for the examples. Without the unstable force_multiline_blocks flag, some of the examples become significantly harder to read. Compare the match event clauses in these two cases:

with nightly:

let event_loop = EventLoop::new();

let window = WindowBuilder::new().build(&event_loop).unwrap();
window.set_title("A fantastic window!");

let mut cursor_idx = 0;

event_loop.run(move |event, _, control_flow| {
    match event {
        Event::WindowEvent {
            event:
                WindowEvent::KeyboardInput {
                    input:
                        KeyboardInput {
                            state: ElementState::Pressed,
                            ..
                        },
                    ..
                },
            ..
        } => {
            println!("Setting cursor to \"{:?}\"", CURSORS[cursor_idx]);
            window.set_cursor_icon(CURSORS[cursor_idx]);
            if cursor_idx < CURSORS.len() - 1 {
                cursor_idx += 1;
            } else {
                cursor_idx = 0;
            }
        },
        Event::WindowEvent {
            event: WindowEvent::CloseRequested,
            ..
        } => {
            *control_flow = ControlFlow::Exit;
            return;
        },
        _ => (),
    }
});

with stable:

let event_loop = EventLoop::new();

let window = WindowBuilder::new().build(&event_loop).unwrap();
window.set_title("A fantastic window!");

let mut cursor_idx = 0;

event_loop.run(move |event, _, control_flow| match event {
    Event::WindowEvent {
        event:
            WindowEvent::KeyboardInput {
                input:
                    KeyboardInput {
                        state: ElementState::Pressed,
                        ..
                    },
                ..
            },
        ..
    } => {
        println!("Setting cursor to \"{:?}\"", CURSORS[cursor_idx]);
        window.set_cursor_icon(CURSORS[cursor_idx]);
        if cursor_idx < CURSORS.len() - 1 {
            cursor_idx += 1;
        } else {
            cursor_idx = 0;
        }
    }
    Event::WindowEvent {
        event: WindowEvent::CloseRequested,
        ..
    } => {
        *control_flow = ControlFlow::Exit;
        return;
    }
    _ => (),
});

EDIT: aaaand, we can't ignore folders on stable. :|

@aloucks
Copy link
Contributor

aloucks commented Jun 22, 2019

I had a hard time spotting the difference even when I put those two examples in side-by-side windows :)

I personally don't think the stable option here is more difficult to read. I actually prefer the the slight vertical and horizontal compression. The increased indentation of the unstable option pushes things further and further rightward.

I would leave rustfmt turned on for everything and use #[rustfmt::skip] where needed (I tend to use it for things like matrix definition when I want columns lined up nicely).

Another option that I'd recommend is to set max_width = 120 (the default is 100), but I guess that's a little more subjective.

@ghost
Copy link

ghost commented Jun 22, 2019

I can't tell the difference between the two at a quick glance either. The stable option looks fine to me. As for the line width, I personally use max_width = 80, but I realize that might not be enough for everyone. I would probably just leave it as the default. I think max_width = 120 is too much when working in a two-column editor layout. Another option probably worth mentioning is newline_style, which can enforce consistent line endings, but this will result in difficult merge conflicts if there's already mixed line endings. All in all, the defaults are very reasonable to me.

@aloucks
Copy link
Contributor

aloucks commented Jun 22, 2019

All in all, the defaults are very reasonable to me.

Agreed

@felixrabe
Copy link
Contributor

felixrabe commented Jun 23, 2019

Another option probably worth mentioning is newline_style, which can enforce consistent line endings, but this will result in difficult merge conflicts if there's already mixed line endings.

The merge trouble for any whitespace-only change like this is easily contained:

  • Apply the change programmatically in a separate git commit on the whole project. (I actually use a script called git-exec that helps apply commits whose messages start with $ ... in that way.)

  • Do the same to any branch before merging. Rebasing is a bit cumbersome but doable: Apply whitespace to base, go to first commit, apply whitespace, then make a commit diffing first+white to second+white. Repeat. (I'm using this strategy regularly – checkout, reset, commit. I wonder if anyone came up with a more fluent way to do this?)

  • This alpha period is a good time anyway to make such "messy" changes.

@Osspial
Copy link
Contributor Author

Osspial commented Jun 23, 2019

Eh, I suppose that's just me being overly picky. I'd be fine with killing the non-stable options for now.

I'd still like to enable merge_imports at some point when that's stable, but that can wait.

@Osspial Osspial mentioned this pull request Jun 24, 2019
1 task
kosyak pushed a commit to kosyak/winit that referenced this pull request Jul 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment