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.

Sign up for GitHub

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

Jump to bottom

large backlog of pull requests #4783

Closed
douggie opened this issue Oct 30, 2023 · 34 comments
Closed

large backlog of pull requests #4783

douggie opened this issue Oct 30, 2023 · 34 comments

Comments

@douggie
Copy link
Collaborator

douggie commented Oct 30, 2023

Hi,

we seem to have a large backlog of PR's?

Any thoughts on how we can get through them and get them merged?

Happy to help out any way I can
@bigscoop
Copy link
Contributor

bigscoop commented Nov 2, 2023

I was thinking of creating the fork where the PR's can be approved (or at least reviewed). Plenty of PR's are still pending from me as well.

Would someone support the idea?

@douggie
Copy link
Collaborator Author

douggie commented Nov 2, 2023 via email

@bigscoop
Copy link
Contributor

bigscoop commented Nov 2, 2023

Unfortunately there is no activity from core team since couple of months. That makes me think of creating the fork

@douggie
Copy link
Collaborator Author

douggie commented Nov 2, 2023 via email

@douggie
Copy link
Collaborator Author

douggie commented Nov 2, 2023

So options

Option 1 - Create a fork with new approvers, this seems a bit nuts, just add more approvers to existing project.
Option 2 - Ensure that a PR has to be reviewed by a contributer before it is reviwed by an approver, this kinda ensures the prioity is self selecting.
Option 3 - Add more approvers to the existing project.

I kinda thinking a combination of 2 and 3.
@timmolter @makarid @npomfret @mmazi what do you guys think?

@rizer1980
Copy link
Contributor

Hello.

I agree that PR is viewed poorly.
Also, important!, somebody need to clear up a huge number of old error reports.
Option 1 - IMHO, bad. Project stratification, unclear prospects.
I vote for 2 and 3

@douggie
Copy link
Collaborator Author

douggie commented Nov 7, 2023

ping @walec51 @timmolter @makarid thougths?

@makarid
Copy link
Collaborator

makarid commented Nov 7, 2023 via email

@bigscoop
Copy link
Contributor

bigscoop commented Nov 7, 2023

@makarid another option would be to extend the list of maintainers. what do you think about it?

@douggie
Copy link
Collaborator Author

douggie commented Nov 7, 2023 via email

@rizer1980
Copy link
Contributor

fork is the last thing.
We need a person who will carry the project so that the majority of users start using its version.
Otherwise, there will be a bunch of forks, which over time will lose any ability to merge them with the main branch.
This will definitely not benefit the project.

I really don't understand why not give a few more people the opportunity to service the project?
yes, the quality may suffer a little, but the project will remain a single whole and will continue to develop...

@douggie
Copy link
Collaborator Author

douggie commented Nov 7, 2023 via email

@douggie
Copy link
Collaborator Author

douggie commented Nov 8, 2023 •
edited
Loading

@makarid are you able to add mainters? May be add me, @bigscoop @rizer1980 as a start? Any others we think?

My start would be

  1. People have to get their PR's reviewed by another contributor
  2. the maintainers do the final reivew and merge.

@makarid
Copy link
Collaborator

makarid commented Nov 8, 2023 via email

@timmolter
Copy link
Member

I'm happy to add more maintainers. Who should I add?

@douggie
Copy link
Collaborator Author

douggie commented Nov 8, 2023 •
edited
Loading

@timmolter pleased you are alive! Great news on adding more matainers, forking the repo seems a bit of e nuclear option.

May be add @bigscoop @rizer1980 and @douggie as a maintainers for a start? This is just based on who has a few PR's out there. @bigscoop @rizer1980 or anyone else any thoughts on who should be added?

@timmolter I am happy to take over the whole maintainer thing (i.e. your role), i.e. ci pipelients etc etc, if you want to hand over the reins fully, we can do a chat some time and share any passwords to switch over users etc etc if I need more access than just maintainer such as adding other maintainers

Douggie

@douggie
Copy link
Collaborator Author

douggie commented Nov 8, 2023

@bigscoop @rizer1980 if you have any pressing PR's can you get them reviwered by another contributer first.

I will review the following PR's as I would like them in
#4771
#4766
#4733

If anyone is able to review some of mine that would be great

#4785
#4780

@bigscoop
Copy link
Contributor

@douggie I think I still can't neither review nor merge anything because of missing role

@walec51
Copy link
Collaborator

walec51 commented Nov 13, 2023 •
edited
Loading

sorry for my absence - I'm not working in the crypto space now and didn't have time to review

we should choose new maintainers based on their experience and availability

looking at https://github.com/knowm/XChange/graphs/contributors

I'm in favor of adding @douggie @makarid @badgerwithagun

as for

@rizer1980 - I'm sorry but I do not see you on the contributors page and you should build more experience before becoming a maintainer

@bigscoop - I see you have your first contributions in the project but I think it would be a little early for a maintainer role, but a couple of more PRs and experience and I think you should be granted that right

@rizer1980
Copy link
Contributor

Hello, @walec51
Hurray!, I understand that we have someone to approve the PR, thx.
I don't understand why I'm not on this page...

I have 16 PRs (11 approved), plus about 10 more ready, but not published for various reasons.
Yes, I don’t have much experience in Xchange, but I wasn’t going to go somewhere I don’t know.
Clean up the backlog of issue, approve the code that I fully understand.

@timmolter
Copy link
Member

@rizer1980 it's because I need to manually update that page during the release process and I may have forgotten recently. You'll be on there on the next release.

@timmolter
Copy link
Member

I've added @douggie and @makarid . @badgerwithagun was already able to merge PRs.

@earce @jheusser @badgerwithagun @walec51 Shall I remove you as being able to merge PRs (write role)?

@timmolter
Copy link
Member

Since it's been a long time since XChange had a release, we should try to get one out soon. I will publish it once I get the green light, and I'll also update the contributor's list at that time.

@douggie
Copy link
Collaborator Author

douggie commented Nov 13, 2023 via email

@makarid
Copy link
Collaborator

makarid commented Nov 13, 2023

Thank you very much @timmolter !

@walec51
Copy link
Collaborator

walec51 commented Nov 13, 2023 •
edited
Loading

I plan to return to an active role next year so it would be nice if I could keep my status.

ps. Lets all remember that we should not merge our own PR - some other maintainer must approve it.

Please also try to maintain best practices described on our wiki: https://github.com/knowm/XChange/wiki

Keep backward comparability and do not let non generic features slip to generic interfaces and DTOs.

@rizer1980
Copy link
Contributor

As about the release.
In my opinion, everything is fine with us, there were reports of problems with [Binance Us] - but now everything seems to be stable.

@timmolter
Copy link
Member

Glad to see renewed activity here! Let's aim for a release late next week. Need to get lots of open PRs approved and merged!

@douggie
Copy link
Collaborator Author

douggie commented Nov 14, 2023 via email

@douggie
Copy link
Collaborator Author

douggie commented Nov 16, 2023 •
edited
Loading

@timmolter not sure I have maintainer access yet, was just going to review and merge some PR's but no luck

Uploading Screenshot 2023-11-16 at 21.06.43.png…

@timmolter
Copy link
Member

Hi @douggie The invite was sent but not accepted. I resent the invite.
Screenshot 2023-11-28 at 10 55 33 AM

@douggie
Copy link
Collaborator Author

douggie commented Nov 28, 2023

thanks @timmolter! I have the merge power now!

Screenshot 2023-11-28 at 09 57 20

@timmolter
Copy link
Member

Great! I hope to cut a new release this week.

@timmolter
Copy link
Member

5.1.1 released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@walec51 @timmolter @douggie @bigscoop @rizer1980 @makarid