-
Notifications
You must be signed in to change notification settings - Fork 277
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] Consider merging PRs proactively rather than en masse just prior to a release #901
Comments
If pysam does move to merging PRs as they come up, you also might consider adding me to the pysam-developers organisation. I would be happy to share the load of reviewing and merging the more trivial PRs, and would find it useful to be able to merge my own infrastructural PRs and changes myself. |
Hi John, Speaking personally, the lag in merging PRs is not as much a policy as a consequence of lack of time to deal with pysam issues on a day-to-day basis. I very much welcome your help and others in the community to help maintain pysam. To address some of this gap, my company hired a full time employee who was to spend the majority of his time working on pysam and related open source projects. Unfortunately, a new and urgent project came up and he has yet to be able to spend much time on pysam. Rather than dwelling on the past, here is my proposal for moving forward:
@AndreasHeger and others please share your perspectives. Thanks, |
Hi @jmarshall , it is good to raise this and I agree with everything that @kevinjacobs-progenity has said. We used to merge PRs as soon as possible, but for me, time has become harder to come by. I have left academia and family commitments mean I have had very little time recently. That being said I would like to remain involved. To echo @kevinjacobs-progenity, there are two ongoing strands of work.
I will mostly focus on 2 for the moment with the primary goal to release an overdue htslib 1.10 based pysam. The idea here is not to add more features and any help will be highly appreciated. @jmarshall , I will add you to the list of maintainers. Please feel free to merge PR as you see fit - many thanks for all your contributions to date! I think a release of htslib 1.10 compatible python by end of March should be doable - merging existing PRs in the next two weeks and reviewing the more serious issues for any low-hanging fruits to be fixed. |
Thanks, I have started by merging a couple 😄 Great, I am happy to help by merging some of the simple stuff and working on the interface between pysam and HTSlib, where I have some expertise. I hope you will have some time to remain involved too (and that having me taking care of some of it will help with that); in particular, I'm not particularly Python knowledgeable and especially I don't know anything about how to make releases so don't want to have anything to do with that! I am more interested in (2) too. I've merged #890 which is a large part of the move to 1.10. My next plan would be to update devtools/import.py a bit (in particular, to drop (or finish dropping in some cases) {htslib,samtools,bcftools}/test and other unused-in-pysam parts of the imported projects) and then the next PR that is ready to go is the bulk of the rest of the 1.10 update. Then we can see to what extent pysam should adjust to htslib 1.10's |
PR updating the imports to 1.10.x now created: PR #905. I'll sit back a bit now while the actual Python experts bash away at testing with this PR… 😄 |
On a slightly different [meta] topic: there are now two reported pysam issues that would be solved by backporting htslib/bcftools fixes from after their 1.10.2 releases — #928 and #941. So we could fix these and make a pysam 0.16.0.2 release. OTOH the upcoming htslib/samtools/bcftools (presumably 1.11) release is said to be imminent, somewhere in the range of 1.5 weeks to a month (I'm guessing). We'd quickly incorporate those and make a 0.17.0 or so release. Do we want to spend the effort to make a 0.16.0.2 release? I'm happy to apply the two backports to master if someone else wants to crank the release-making wheels. |
The current pysam policy is to merge PRs in a flurry just prior to making a new release. (Also there is not much indication of when such releases are imminent.) This makes development more difficult, as there is not much building from and testing of master to allow new code to be exercised and bedded in. It probably also slows development and discourages contributions from non-core developers.
IMHO it would be better to review and merge PRs more proactively, as individual PRs are opened rather than eventually together as a batch. This would make the main branch reflect the current state of development, simplify dependent development, and generally reflects best practices.
A case in point is PR #890, which fixes the pysam build when used with samtools 1.10 released last December. The build is currently broken when used with an increasingly common external samtools command. Also I have a number of other pull requests that are followups to this one, and the logistics of this are difficult when the base PR has not been merged.
CC @AndreasHeger @bioinformed @kyleabeauchamp
The text was updated successfully, but these errors were encountered: