search

koalalorenzo / python-digitalocean

🐍🐳 Python module to manage Digital Ocean droplets
GNU Lesser General Public License v3.0
1.25k stars 300 forks source link

Implement the firewall rules management #358

Closed aliasifk closed 1 year ago

aliasifk commented 1 year ago

Can add/remove outbound and inbound rules

aliasifk commented 1 year ago

@koalalorenzo Can you please merge, I have written all the test cases.

scy commented 1 year ago

@aliasifk I have contributed one single PR to this project back in 2020, and yet you’ve contacted me via email & LinkedIn in order to get this PR reviewed.

Please don’t do that. I’m not sure whether getting your PR reviewed by random people on the internet will result in it being merged any faster. But also, nagging folks via email and social media is frowned upon and they’re more likely to consider you a nuisance than actually helping you out.

Also, your PR is only 4 days old. This is an open source project, people maintain it in their spare time. It can take weeks or months for things to get merged. If you need the feature you’ve added sooner, consider using your fork instead.

aliasifk commented 1 year ago

I am really sorry that I disturbed you, I had no intentions of doing it .I am relatively new to PRs and I did not knew It took time and was excited enough to email. I just looked at your profile and sent a connect on LinkedIn not because I wanted you to PR but I found your profile interesting . I also did not mail “folks” just you and the owner of repo .You because I saw your name in the repo and thought maybe you review PRs. Once again, sorry.

And as I mentioned, its a learning experience for me and now I understand how PRs and open source projects works. I have forked the repo and released my own package, and once whenever koalalorenzo gets time, he can review it. Until then I can use my own package

koalalorenzo commented 1 year ago

Thank you for your contribution, it is appreciated. Though, do not reach out to us privately insisting on getting a review. We are not getting paid for this project. As I told you, sadly I have very little time on my hands and bugging people around is not helping making the process easier.

I will evaluate this PR later when I will have time.

koalalorenzo commented 1 year ago

@aliasifk I can see that the code is very much looking like #244 but with the addition of the tests.

What I can see is that part of your code looks copied, pasted and "re-authored", and instead of rebasing it and attributing code ownership to @sajadbahar, it has been re-written. It would be nice to see your code, rebased on top of #244 so that both you (@aliasifk) and @sajadbahar will be seen as contributors, instead of just you.

A good suggestion for OSS contribution is to not steal other people code and rebrand it as yours! 😅 Unless you and @sajadbahar are the same person/legal identity! 🤣

What I would suggest is one of the following:

Probably the first option would be ideal! 💪 Please let me know and thank you again for your help with the tests! 😄

aliasifk commented 1 year ago

@koalalorenzo Thanks, yes I first saw the PR that he has done and definitely copied those methods and added the test cases, because I thought the PR to be too old and did not saw any further comments from their side when you had suggested changes to add test cases So I decided to create a new PR. But let me see if I can reach out to him or perhaps do something to involve his contribution, among the options you have suggested

sajadbahar commented 1 year ago

@aliasifk I have rebased my fork with the master and it's good to go, you could get the latest code and create PR with your change on top of #244

@koalalorenzo Thanks for your comment

PS: @aliasifk Don't copy and paste someone's code and make it yours

aliasifk commented 1 year ago

Ok I have created a new PR #359

Also just clarifying, https://github.com/koalalorenzo/python-digitalocean/pull/358#issuecomment-1261993912

@sajadbahar if you read the comment, I said "added tests" and did not make the code "mine" Once again, it was definitely mistake on my behalf of assuming that the PR is too old and copy pasting the commits, should have at least mentioned it in the commit message but I have not experienced such a scenario until now, I apologize on my behalf for being ethically wrong and I definitely learnt some stuff here

At the same time, @koalalorenzo thanks for taking your precious time to continue to suggest changes, but I thought mentioning words like "steal" was not very appropriate.

Thanks.