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

Maintainer Roll-call? #236

Open
ssfrr opened this issue Oct 24, 2018 · 15 comments
Open

Maintainer Roll-call? #236

ssfrr opened this issue Oct 24, 2018 · 15 comments
Labels

Comments

@ssfrr
Copy link
Contributor

ssfrr commented Oct 24, 2018

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.

@martinholters
Copy link
Member

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?

@rob-luke
Copy link
Member

Sounds like a good plan. I can make more of an effort too.

@galenlynch
Copy link
Member

galenlynch commented Oct 24, 2018 via email

@ssfrr
Copy link
Contributor Author

ssfrr commented Oct 26, 2018

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 conv and filt with independent implementations).

@ararslan
Copy link
Member

@galenlynch You should now have received an invitation to join the organization.

@galenlynch
Copy link
Member

Thanks, I accepted the invitation!

@galenlynch
Copy link
Member

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?

@ssfrr
Copy link
Contributor Author

ssfrr commented Oct 26, 2018

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 master, so it will end up being the next breaking release, but if we want to backport things to additional backwards-compatible 0.5.x releases we can branch off of the previous tag.

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.

@sirtom67
Copy link
Contributor

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.

@jfsantos
Copy link
Member

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.

@ararslan
Copy link
Member

Good luck on your defense!

@rob-luke
Copy link
Member

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?

@martinholters
Copy link
Member

I'd say: Approve, wait an appropriate amount of time whether anyone mentions objections, merge if none.

@ssfrr
Copy link
Contributor Author

ssfrr commented Feb 17, 2020

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.

@rob-luke
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants