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

Refactoring #3

Merged
merged 10 commits into from
Jun 16, 2023
Merged

Refactoring #3

merged 10 commits into from
Jun 16, 2023

Conversation

NicKoehler
Copy link
Collaborator

I'm interested in helping with this library.

I followed this to implement more functionalities about posts.
Also I added types and some docs to post methods.

Fell free to review my changes to your api if you want me to change something.

@db0
Copy link
Owner

db0 commented Jun 15, 2023

Thanks for helping!

One suggestion what do you think about making the post.__call__() effectively call .create()? This way one can still call lemmy.post().

What I don't quite like is copying the authentication values into every class. I'd rather put the authentication into its own class singleton and have every class import it, or something similar.

@NicKoehler
Copy link
Collaborator Author

What do you think of something like this?

@db0
Copy link
Owner

db0 commented Jun 15, 2023

Assuming you're tested it, it looks good to me. Just change the post class to Post to be CamelCase as per python naming suggestions.

@NicKoehler
Copy link
Collaborator Author

I didn't test everything, I need to create a dummy instance first, I'll test it and let you know

@db0
Copy link
Owner

db0 commented Jun 16, 2023

LGTM. Mark it as ready when you're done

@NicKoehler NicKoehler marked this pull request as ready for review June 16, 2023 12:48
@db0 db0 merged commit 06017f4 into db0:main Jun 16, 2023
@db0
Copy link
Owner

db0 commented Jun 16, 2023

Great job. I've sent you an invite as collaborator so you can push new versions.

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

Successfully merging this pull request may close these issues.

2 participants