uber / piranha

A tool for refactoring code related to feature flag APIs
Apache License 2.0
2.29k stars 198 forks source link

Add support for ruby for polyglot piranha #649

Closed Bennet-Sunder closed 6 months ago

Bennet-Sunder commented 8 months ago

This PR adds support to clean up ruby code in polyglot piranha.

ketkarameya commented 8 months ago

@dvmarcilio please let me know if adding this support is alright, and ya'll are ready to maintain it as FF cleanup track.

przemyslawbialon commented 8 months ago

Hi, what's the status of this PR? is there any chance it will be merged? What is the expected timeline? I'd love to run a pov in my organization but we need support for ruby.

ketkarameya commented 8 months ago

Hey! Thank you for reaching out. I am not leading this change, so I have no idea when ruby support will be there. Currently, as a repo we do not have any priority to add ruby support, because uber is not a big ruby house. But we are open for contributions. Actually, Idk if folks at Uber want this, and want to prioritize this. @dvmarcilio thoughts? Is this on your agenda? If not, we can still review and land this diff whenever it is ready?

Bennet-Sunder commented 8 months ago

@przemyslawbialon This PR is almost done and will be ready for review in a week or two. In the meanwhile if you wish to evaluate it, you will be able to get that done by cloning this branch and trying it out locally.

dvmarcilio commented 8 months ago

Sorry for the delay. As @ketkarameya said, this is not our priority. However, community PRs are always welcome and we appreciate @Bennet-Sunder 's work. We're happy to review it given this check all the boxes, including adding comprehensive tests, just as the rules for the other languages.

Bennet-Sunder commented 7 months ago

@ketkarameya @dvmarcilio I have added the tests for the changes, could you pls review the PR when you have bandwidth.

ketkarameya commented 6 months ago

@donald-pinckney please shepherd this PR

donald-pinckney commented 6 months ago

Hi @Bennet-Sunder, all of this looks like amazing work for Ruby! Let's get this merged in, and any additional useful cleanups can always be added later :)

ketkarameya commented 6 months ago

Thanks for your contributions @Bennet-Sunder

Bennet-Sunder commented 6 months ago

Thanks @donald-pinckney @ketkarameya for your time in reviewing and merging this PR :)

ketkarameya commented 6 months ago

I merged it but it failed on main. I think the reason is something similar to a discussion u had started. Could tell me how u got about it?

donald-pinckney commented 6 months ago

PR #663 should fix the broken build introduced by this PR.

Bennet-Sunder commented 6 months ago

@przemyslawbialon Support for ruby is now merged and should be available in the next release.