-
Notifications
You must be signed in to change notification settings - Fork 950
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
[META] Add a CONTRIBUTING.md #674
Conversation
bf631e0
to
36fa91d
Compare
I feel like an official list of supported/targeted features is important enough to get its own file in root (perhaps Also, as far as tier-1 features go, there are a few things I'm not seeing on the list in the issue:
Where do we stand on those? AFAIK there was hesitation about gamepad support, the windowing features haven't received much attention, and gyroscope support hasn't received any attention despite being common on mobile devices. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome stuff @vberger, thanks for taking the time to put this together!
Other than a couple typos I'm happy to see this land :)
who exactly is a "maintainer" of winit?
I guess everyone contributing PRs, reviewing PRs, testing them and providing feedback are helping with maintenance in their own way, however as for the "lead" maintainers (those with an M in the table) it is admittedly trickier to hone down a definition.
E.g. I would consider you the clear maintainer of Wayland in that you have both merge access, are a frequent reviewer and contributor and have the most extensive knowledge of the backend.
Myself on the other hand, I feel I have a very jack-of-all-backends position in that I'm vaguely familiar with and have contributed a fair share to all of macOS, Linux and Windows, yet I am certainly not an expert in any of them and wouldn't dare claim the lead M
role for any. That said, I have merge access and am willing to review, test and merge (any that I have access to at the time) under the condition that the two maintainer policy is met.
CONTRIBUTING.md
Outdated
Wayland, Android, iOS and the web platform via Emscripten). | ||
|
||
Most platform expose capabilities that cannot be meaningfully transposed to the others. Winit does not | ||
aim at supporting every single functionnaly of every platform, but rather abstract the set of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functionnaly -> functionality
CONTRIBUTING.md
Outdated
|
||
Once your PR is open, you can ask for review by a maintainer of your platform. Winit's merging policy | ||
is that a PR must be approved by at least two maintainers of winit before being merged, including | ||
at least a maintainer of the platoform (a maintainer making a PR themselves counts as approving it). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
platoform -> platform
@Osspial I like your idea of having a SCOPE.md file! It always takes me longer than I'd like to hunt down the the feature table issue. Having a dedicated file would make it easier to find, review and make updates. Perhaps we could even require that changes to it be mentioned in the CHANGELOG.md. Re tier 1 features: Maybe we can leave this discussion for a future PR/issue dedicated to this. It might be easy to unintentionally derail @vberger's CONTRIBUTING.md if we start the discussion here :) |
@Osspial 👍 for making a @mitchmindtree Do you think it may be worth adding a third level, between
In general I think it'd be best to keep the number of levels as low as possible to keep things crystal clear, but I guess 3 levels like this is still reasonably simple. |
CONTRIBUTING.md
Outdated
Winit aims to provide a generic platform abstracting the main graphic platforms (Windows, MacOS, x11, | ||
Wayland, Android, iOS and the web platform via Emscripten). | ||
|
||
Most platform expose capabilities that cannot be meaningfully transposed to the others. Winit does not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/platform/platforms
CONTRIBUTING.md
Outdated
Wayland, Android, iOS and the web platform via Emscripten). | ||
|
||
Most platform expose capabilities that cannot be meaningfully transposed to the others. Winit does not | ||
aim at supporting every single functionaly of every platform, but rather abstract the set of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/aim at supporting/aim to support
s/functionaly/functionality
s/rather abstract/rather to abstract
CONTRIBUTING.md
Outdated
different "support levels": | ||
|
||
- Tier 1: features which are in the main scope of winit. They are part of the common API of winit, and | ||
are taken care of by the maintainers, and them not working correctly is considered a bug in winit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/are taken care of by the maintainers, and them not working correctly is considered a bug in winit./are taken care of by the maintainers. Any part of these features that is not working correctly is considered a bug in winit.
CONTRIBUTING.md
Outdated
|
||
- Tier 1: features which are in the main scope of winit. They are part of the common API of winit, and | ||
are taken care of by the maintainers, and them not working correctly is considered a bug in winit. | ||
- Tier 2: some platform specific features can be sufficiently fundamental to the platform that winit can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/platform specific/platform-specific
CONTRIBUTING.md
Outdated
When making a code contribution to winit, before opening your pull request, please make sure that: | ||
|
||
- you tested your modifications on all the platforms impacted, or if not possible detail which platforms | ||
were not tested, and what should be tested, so that a maintainer or an other contributor can test them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/an other/another
Left a few copy notes, I think this is great! |
@vberger I'm fine with splitting the scope discussion off. If we're doing that, should we move most of the current I also like the idea of a reviewer tier in the maintainer/tester table. |
I don't have a strong opinion on this. Probably splitting this part of to a |
@vberger That sounds good. For now though, I'd say to leave that part in and then have the |
@Osspial Alright, sounds good to me. Let's forward this and then open a large PR/discussion about defining the exact scope of winit. @francesca64 Given you've been in charge of winit for some time, what's your opinion on this suggested Also, @francesca64 @mitchmindtree and @Osspial , assuming we keep the |
@vberger For me, it would conservatively be this:
Also, I don't know all that much about Emscripten but I do have the compiler set up for it and it seems to be small enough that if we need a reviewer for it learning about would be a doable task! |
Feel free to put me down as T for X11, Wayland and Windows (though I only have access to windows via a VM). |
@vberger this
In an ideal world, I'd only be the main maintainer for X11, so it could be good to have a note saying that macOS, Android, and iOS are still up for grabs. @Osspial I'd be quite happy if you could take over the Emscripten backend, and I don't just say that because it's the only one I never managed to run. It's definitely the one I know the least about, and it would make the I think you'd do a great job as maintainer of the Windows backend, and I'll email @tomaka about making you a collaborator if he doesn't show up here. |
I would like to help with iOS/Android. My company has 2 devs (mostly me) actively developing a 2D game/game engine in rust using gfx-ll. I'm currently focused on iOS, and have made some contributions there (not recently). I'm intending to get rid of the longjmp hacks and port it to EventLoop 2.0 as my next task. I'm up for maintaining iOS, and atleast able to test macos/android, but I haven't messed with that code much. |
Given all this seems now pretty consensual, I'm going to merge this as it is now. @Osspial Can you open the @mtak- Can you then open a new PR adding yourself to the table, so that we can discuss there? I have nothing to oppose you taking the maintainership of the iOS backend, but I'd rather not delay this PR any longer. Thanks for proposing your help! |
This PR introduces a
CONTRIBUTING.md
file based on the discussion in #669. This is for now very WIP and intended to serve as a basis for discussion, like an RFC.Regarding the most fundamental still-open questions:
Rendered version of the file: https://github.com/vberger/winit/blob/contributing/CONTRIBUTING.md