-
-
Notifications
You must be signed in to change notification settings - Fork 40
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
Hello? <crickets> #78
Comments
Hey @JalonSolov, Right now there are no bigger changes planned for this project. We aim to do everything in the open: Discussions, bigger refactorings, pull requests, branches, etc. I am (at the moment) not using this library actively in projects anymore. This might be the reason why this library is lacking a few features or is not following 100% the progress of Gerrit. I would be happy to accept new contributors and PRs to make the things happen that you describe. To avoid long ping-pong in PRs, I think a small design discussion on how to structure bigger reworks might be useful. An issue can be a good platform. In terms of bigger changes: I think we should follow best practices here and keep the changes as small as possible as long as they fit together. This ensures a quick review + quick merge. What do you think about this? |
Small changes are better, unless a large change is necessary. Keep the Unix philosophy of "do one thing and do it well" as much as possible, but don't hamstring changes to force them to be small. In that same vein, I try to write my own code so that it "just works". The user of my code shouldn't have to worry about which version of something else they are using, my code should continue to work. If I can't make it do that, I give a message telling them what to do instead. To do that with this library shouldn't be too hard. You already have a function to get the version of Gerrit, so just have the library get that for itself and store it. Have this routine also check that version and complain if the Gerrit version is either too low or too high, and suggest using a different version of the library in either case. Then you can have "if Gerrit version X do this, else if Gerrit version Y do this". When that starts making the code too complicated, do a final push to set the high Gerrit version in the current library, then create a new version of the library. It really isn't too complex to add this type of code, and the overhead of getting the Gerrit version isn't very high, so it all "just works" without impacting the user very much. And you also don't have to follow the same versions as Gerrit, just know which versions the library works with. For a more concrete example of this, the new version check routine could check for minimum version of 2.11.3-1230-gb8336f1 (the version you used it with), and maximum version of... whatever it gets adjusted to work with. If it is too complex to make it work with Gerrit 2.x and Gerrit 3.x at the same time, there's the answer for when to make a new version of the library. How does that sound? |
Sounds magical and awesome! |
Hey @JalonSolov, |
Nothing worthy of a PR, yet. Other priorities have intruded. |
Hey @JalonSolov, |
Hopefully everyone is alive and well, no COVID-19 infections, etc.
I was just wondering if any future changes are planned on this project? Yes, I know, I can fork it, make my own changes, submit PRs, etc., but I would hate to waste time doing that if big changes were already in the works.
Basically, there have been many, many API changes since Gerrit 2.11. Current release is 3.1.3. I ran into one "breaking" change just trying to use this library with Gerrit 2.14:
'DELETE /changes/{change-id}/reviewers/{account-id}/votes/{label-id}'
'POST /changes/{change-id}/reviewers/{account-id}/votes/{label-id}/delete'
To delete a vote and send options (such as "notify: none"), you must use the POST method now. It used to work with the DELETE method, but they changed it to be more "REST-like".
They have also added several new REST endpoints.
All that said, this library helped me greatly with a project I was working on, so kudos there. :-)
So... if nobody is working on this library, I may give it a shot. Just wanted to see which way things are headed.
The text was updated successfully, but these errors were encountered: