search

scikit-hep / probfit

Cost function builder. For fitting distributions.
http://probfit.readthedocs.io/
MIT License
51 stars 30 forks source link

fix: build system update #105

Closed henryiii closed 3 years ago

henryiii commented 3 years ago

Closes #78. Closes #97. Follows the Scikit-HEP developer guidelines.

TODO: activate Read-the-docs PR mode so we don't have to build that in CI.

henryiii commented 3 years ago

Who owns RtD for this? It needs to have build-on-PRs turned on.

henryiii commented 3 years ago

I'll check the wheel build after this gets merged. Eh, I can do it now. Also, what platforms are supported? Windows too?

henryiii commented 3 years ago

@HDembinski @piti118 @cdeil or @alexpearce, please add scikit-hep as a maintainer or owner for this package on PyPI. All Scikit-HEP packages should have scikit-hep listed. Thanks!

henryiii commented 3 years ago

Okay, ready.

henryiii commented 3 years ago

The badge needs to be updated to GHA

eduardo-rodrigues commented 3 years ago

@HDembinski @piti118 @cdeil or @alexpearce, please add scikit-hep as a maintainer or owner for this package on PyPI. All Scikit-HEP packages should have scikit-hep listed. Thanks!

+1: that would be great as many of us can then manage the package.

eduardo-rodrigues commented 3 years ago

@HDembinski @piti118 @cdeil or @alexpearce, please add scikit-hep as a maintainer or owner for this package on PyPI. All Scikit-HEP packages should have scikit-hep listed. Thanks!

@henryiii I created a separate issue (https://github.com/scikit-hep/probfit/issues/106) for this, to move forward.

eduardo-rodrigues commented 3 years ago

Who owns RtD for this? It needs to have build-on-PRs turned on.

I guess at least @HDembinski ?

eduardo-rodrigues commented 3 years ago

BTW this PR also closes https://github.com/scikit-hep/probfit/pull/103.

HDembinski commented 3 years ago

@HDembinski @piti118 @cdeil or @alexpearce, please add scikit-hep as a maintainer or owner for this package on PyPI. All Scikit-HEP packages should have scikit-hep listed. Thanks!

+1: that would be great as many of us can then manage the package.

I invited scikit-hep on PyPI two days ago when I saw the message. It says invitation pending.

HDembinski commented 3 years ago

Regarding RTD, I have the rights to add more maintainers. Which user should I invite?

HDembinski commented 3 years ago

I am perfectly happy to merge this, but I am not considering myself as the maintainer of this package. I haven't looked into it for a while. In other words, there is no need to wait for me to merge this, just merge as you see fit.

eduardo-rodrigues commented 3 years ago

@HDembinski @piti118 @cdeil or @alexpearce, please add scikit-hep as a maintainer or owner for this package on PyPI. All Scikit-HEP packages should have scikit-hep listed. Thanks!

+1: that would be great as many of us can then manage the package.

I invited scikit-hep on PyPI two days ago when I saw the message. It says invitation pending.

Thanks very much @HDembinski. I got no notification and had not checked. Have just logged in and accepted the invitation. Sorted.

eduardo-rodrigues commented 3 years ago

I am perfectly happy to merge this, but I am not considering myself as the maintainer of this package. I haven't looked into it for a while. In other words, there is no need to wait for me to merge this, just merge as you see fit.

Sounds more than fair. Indeed there is no maintainer per se anymore. On the other hand you do have/had certain admin rights, hence the messages to you.

eduardo-rodrigues commented 3 years ago

Regarding RTD, I have the rights to add more maintainers. Which user should I invite?

Maybe add @henryiii since he is doing all the updates here?

HDembinski commented 3 years ago

Maybe we need some community rules for packages like this. If there are two scikit-hep people, the proponent of a change and another, approve the change, it can be merged by the proponent?

HDembinski commented 3 years ago

Regarding RTD, I have the rights to add more maintainers. Which user should I invite?

Maybe add @henryiii since he is doing all the updates here?

Done

eduardo-rodrigues commented 3 years ago

Maybe we need some community rules for packages like this. If there are two scikit-hep people, the proponent of a change and another, approve the change, it can be merged by the proponent?

I pretty much do like that for work with Henry on Particle, DecayLanguage, etc. In fact, if I review and it is a no-brainer I do the merge myself straight away most of the time; else if there is just a little question or update I approve, Henry makes the update to his PR and merged once the CI is again green. In short, I agree with your pragmatic rule and think we mostly follow it implicitly everywhere where more than 2 person is a main author and/or maintainer.

HDembinski commented 3 years ago

Closes #97

henryiii commented 3 years ago

Thanks everyone!

Maybe we need some community rules for packages like this. If there are two scikit-hep people, the proponent of a change and another, approve the change, it can be merged by the proponent?

That's the unofficial way that we tend to work on "community maintained" packages. Might be better to make it a bit more formal and explicit; though so far, it's mostly @eduardo-rodrigues and myself doing community maintenance. For packages with one or more dedicated maintainers (iMinuit, boost-histogram, uproot, mplhep, etc), then lead maintainer gets final say (except in an emergency like a broken release and they can't be reached? But so far all our maintainers have been good about avoiding releasing broken packages and have been very accessible).

eduardo-rodrigues commented 3 years ago

Makes sense, @henryiii.