-
Notifications
You must be signed in to change notification settings - Fork 110
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
Maintainer Roll-call? #236
Comments
I'd also like to devote some more time to DSP.jl, but don't see that coming in the near future. However, if anyone feels like needing another pair of eyes somewhere, feel free to ping me. I'm also absolutely fine with adding @galenlynch. BTW, who has the rights to do so? |
Sounds like a good plan. I can make more of an effort too. |
I'd be happy to help out! Are there guidelines for maintainers?
…On Wed, Oct 24, 2018, 5:01 PM Robert Luke ***@***.***> wrote:
Sounds like a good plan. I can make more of an effort too.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#236 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGg8z1P3mnX5A_p-NH86UfT5eP2wL6Dtks5uoNUdgaJpZM4X3FN8>
.
|
Great - @ararslan, I think maybe you're the one with real ultimate admin power to add maintainers? @galenlynch we don't really have any formal guidelines (though if anyone wants to propose some coding standards, architectural overviews, that sort of thing, it would be welcome). I think DSP.jl is pretty widely used (someone who's run a reverse dependency listing can verify), so we definitely want to be pretty conservative with breaking changes and include deprecation periods, and be particularly careful about anything that would change the result of a computation without throwing an error (for instance if we fix #186 there'd need to be a deprecation period and major version bump on release, which isn't something we want to do very often). That said, there are parts of DSP.jl that have been around a very long time, and things have shifted around with things being moved out of Base and into DSP.jl (and FFT stuff into FFTW.jl), and there might be some opportunity for some cleanup. (e.g. #202, and also something smells fishy about having both |
@galenlynch You should now have received an invitation to join the organization. |
Thanks, I accepted the invitation! |
In practice, how are breaking changes integrated into the repo? For example, #153 seems like a simple PR (albeit a stalled one) with improved defaults, but it changes the API. From my understanding, merging it would mean the next tagged version would be a major release, right? Is there any way to merge breaking but helpful changes and save them for a later major release? |
As it stands (pre 1.0), minor versions serve the function of major versions (e.g. if there are breaking changes the next release should be v0.6.0). At some point we should bite the bullet, release 1.0, and then do real semver, but I suspect there might be a few more changes we want to make before doing that. I think the standard way to approach breaking changes is to do them on If we get some momentum we could even do some release planning by creating a milestone and tagging issues for the next release, but I suspect that might be more overhead than it's worth for now. Just cleaning up the backlog of waiting PRs and trivial issues would be a great start. Maybe that's what we target for 0.6 and then start talking about what we'd want to see in a 1.0 release. |
I would also like to help out when I can. Sorry I've been gone for a couple months. There is way more we could do here in this repo. |
I have been away for ages, but I'm defending my thesis soon and planning on doing some Julia development again. I will be able to help in the near future. |
Good luck on your defense! |
Ok I am back after some time off. Whats the current procedure/etiqute with merging PR's? For simple PRs like #342 I am happy to merge, or do we require multiple approvals? Is it best to just comment that it looks good to me, or do we go ahead and merge? |
I'd say: Approve, wait an appropriate amount of time whether anyone mentions objections, merge if none. |
In general I'd say if we can get two maintainers to look at each PR that's great. If it seems pretty low-risk I'm happy for folks to just merge right away to keep things moving for everyone. If the PR is from a DSP maintainer I'd say that counts as an approval from them (unless it's listed as WIP), so it's probably OK for a second maintainer to just merge in that case rather than the two-step process of approving first. I'm generally in favor of merging more PRs rather than letting them languish (though I'm often guilty of that too). Particularly when errors are likely to be obvious, we can always revert. Definitely PRs that seem like they might cause silently-wrong answers need more scrutiny. |
Great. It helps to have this clear, as different projects do it differently. I’m also going to go back and close out some old issues that seem dead. |
Hey DSP folks.
I have commit access and have reviewed/merged some PRs, so I guess that makes me one of the DSP maintainers. That said I've been pretty derelict in my maintainer duties with a number of meatspace responsibilities. I apologize for that, but to be honest I don't see it changing much in the near future.
I wanted to check in and see who else is still active. I also wanted to propose adding @galenlynch (if he is interested). He's had a number of nice contributions and also had PRs that have sat too long. Any other nominations?
Also in the interest in keeping things moving a little faster, I would encourage other maintainers to merge PRs rather than approving them (though obviously not if you feel iffy and want a 2nd opinion). I know in an ideal world we'd have multiple people checking all PRs, but I think it's worse for good PRs to sit unmerged for months.
The text was updated successfully, but these errors were encountered: