ManderaGeneral / generalimport

Handle all your optional dependencies with a single call!
https://pypi.org/project/generalimport/
Apache License 2.0
16 stars 1 forks source link

Contributions accepted? #18

Closed masci closed 1 year ago

masci commented 1 year ago

Very nice project, congrats!

I see in the project metadata that contribute is set to False - is this going to change in the future?

Mandera commented 1 year ago

Thank you! I'm stoked you find it interesting!

Yes, that's the plan! There are a few things I need to decide on beforehand, all the legal stuff is a bit overwhelming

Mandera commented 1 year ago

Thank you for your interest and contributions @ZanSara! (#19, ManderaGeneral/generalimport#20, ManderaGeneral/generalimport#21, ManderaGeneral/generalpackager#86) Please see this issue - I've moved it up my priority list so please stay tuned.

I will get to your contributions once this is complete. I'd like to apologize for the inconvenience, you guys are the first contributors for me!

Current state

ZanSara commented 1 year ago

Hey @Mandera! Happy to help :blush: I really like the approach you're taking with this library so I'm more than willing to contribute bugfixes and small features.

Me and @masci would love to use this library to manage optional imports in Haystack, which has tons of dependencies and has always struggled with how to manage this growing list effectively. So you can expect some activity from us (and maybe other colleagues of ours) in the next weeks. Apart from the management of exceptions, I know already that some of our most complex dependencies (specifically sqlalchemy, but there are surely others) currently don't work with generalimport, so I will contribute with the bug's descriptions as issues, and later bugfixes for those as soon as I have them. I hope you don't mind me being a bit proactive and going ahead with bugfixes as soon as I need them.

Depending on how you feel about this, I am also willing to work some more "contributor friendly" features: linting and code formatting, pre-commit hooks, API docs on GitHub pages, a pyproject.toml file, a CI with tests, and so on. I will open a separate issues where I can propose some ideas on this front and you can choose which ones you like and which ones you don't.

Regarding the license, for now either (MIT, Apache2) is fine. Regarding copyrights, for us a CLA would work if that simplifies your life (in Haystack we have a bot that asks contributors to sign it and it works quite well), but I'm happy to wait for your decision either way :slightly_smiling_face:

Mandera commented 1 year ago

Wow that's awesome! I'm thrilled you're looking to implement generalimport!

Most of these things would go through generalpackager which is my custom build system that runs the tests and synchronizes everything upon success. You can see the entry points here in the workflow workflow_unittest and workflow_sync.

The current state of generalpackager is not great. I've only been using the master branch so now I'm in the middle of a breaking update in it. I will see if I can move that new stuff to a new branch and create a new workflow like you were talking about too.

Because in the workflow it's cloning every repo to sync it all, so if master has breaking changes it will just fail - not ideal haha!

Github also had to update their SSH keys so that broke the workflow as well, needing some love!

I'd like to probably protect the master branch and only allow reviewed pull requests, once all tests pass.

Currently every package in ManderaGeneral is tested and synced on every commit (Unless message starts with [CI SKIP]). If any of the versions are bumped then it will bump any package that has had non-aesthetic changes to it too. When a bumped package is being synced it will get tagged and published to pypi.

I'm planning to put all this information in a CONTRIBUTING.md eventually.

The long-term goal is to generalize generalpackager to work for any environment, not only ManderaGeneral, but that's far away.

To summarize; I'm loving your contribution ideas but a lot of them will have to go through generalpackager - I also want to finish the legal stuff before accepting contributions

ZanSara commented 1 year ago

Understood! Thank you for the explanation. I'll open a few separate issues to keep track of the things we can address, or at least discuss, while you sort out the rest (namely linting, sqlalchemy, api docs). We can definitely use generalimport already without any of these things though, so the one I would give the most priority to is the contributions/license.

So here is what I know about our handling of contributions (@masci will correct me if I'm wrong):

Do you require every single commit to be signed in Haystack? Or is the CLA enough?

The CLA is enough, we don't require signed commits.

The CLA assistant seems pretty nice, so is that a one-time sign for every significant contributor?

Yes, CLAssistant checks if the contributor has ever signed the CLA before and if not, posts a message on the PR thread asking them to press a button and sign. None ever complained, except maybe a couple of people with really misconfigured GitHub accounts :sweat_smile:

Does the CLA in your example transfer the copyright to Haystack, or does the contributor still keep the copyright?

Great question! I would tell you, but I don't know it myself :smile: I'm going to try find the agreement and let you know.

ZanSara commented 1 year ago

By the way: I might have found out why sqlalchemy caused me trouble: might be not generalimports fault. I'll open an issue only if I'm sure it's due to this library, which now seems more unlikely.

Mandera commented 1 year ago

Alright cool! I've made some decisions and issues

masci commented 1 year ago

Hey @Mandera thanks a lot for your support!

Does the CLA in your example transfer the copyright to Haystack, or does the contributor still keep the copyright?

Also for @ZanSara: yes, by signing the CLA contributors transfer their copyright to deepset. This is commonly done to allow companies to easily manage legal disputes or implement a license change without collecting consensus from every single contributor (reason why CLAs can be a show stopper for many potential contributors).

Mandera commented 1 year ago

Contributions are enabled and the license has been changed! The other changes can happen periodically