keboola/sapi-python-client

Contribution guide

Closed this issue · 9 comments

pocin commented

Hi guys, seems awsome and I'd love to contribute my few cents.
Is there a recommended workflow for contributions? I'd like to ask b4 I start working on a feature.

Right now I have some refactoring ideas. Would you prefer discussing them in advance or should I just make the changes and discuss them in a PR?

Mainly I would

  • rename the methods of the HttpHelper class from camelCase to pep8_case
  • make the Client class a sublcass of the HttpHelper to avoid the awkard calls such as client.http.get_something() (The cleaner version would be client.get_something().
  • some minor changes (like turning the HttpHelper.tokenheader() into a @property. Imo it makes more sense to use it like HttpHelper.tokenheader

This would change the api alot and break lot of existing code so I'm asking in advance if you are ok with that.

That's what I got just through quickly skimming the source, feel free to disagree with anything 🍺

Hi @pocin , welcome!

As far as contribution workflow, I think we can probably at this point get away with just using the PR
comment/review system.

I'm not a python person per se (as is probably obvious by the source:) so there isn't really any strict way I have in mind of how this client should be, other than that it should be a client for the keboola storage api (http://docs.keboola.apiary.io/#).

As for your 3 suggestions above, yes, yes, and yes:) and thank you!

Looking forward to making this thing pleasant and useful together, cheers 🍺

fwiw, I tagged it as 0.0.1 so we can properly version for breaking changes.

i'd just add that it would be nice to keep the method names (and arguments) aligned with the API, and ideally with the PHP client https://github.com/keboola/storage-api-php-client/blob/master/src/Keboola/StorageApi/Client.php so that there is a e.g a listBuckets (or list_buckets to be PEP8) method and not getBuckets method.

pocin commented

@pivnicek (I suppose you have write access to this repo) - can you make a dev branch so we don't have to merge directly into the master? Once the dev is mature enough, we can "release" a new version.

@pocin you should have an invite link in the mail to join the repo with write. I've also sent it to you in a slack PM just in case.

Once you join, please by all means make a branch and a PR whenever you like.

FYI: i made a couple of issues and also a milestone 1.0.0 - i consider the issues for that milestone to be important for real reliable production use. If i forgot something, feel free to add. I do not consider full API coverage to be important for milestone 1.0.0, but maybe some parts might be worth?

@odinuv, I notice you use a nice commit message structure, like tag: description. Do you have documentation for that style of commit message? I might try using it too.

I tihnk I'm using these https://seesparkbox.com/foundry/semantic_commit_messages I'm not overly strict about it thoug :)

materialized in a361476