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

feat(6178): emit ruby enum as integer #11673

Closed

Conversation

MQuy
Copy link
Contributor

@MQuy MQuy commented Jan 26, 2023

Fixes #6178

  • Add a new option enums_as_integers to emit enum as integer value.

@MQuy MQuy requested a review from a team as a code owner January 26, 2023 16:29
@MQuy MQuy requested review from JasonLunn and removed request for a team January 26, 2023 16:29
@JasonLunn
Copy link
Contributor

Thanks for the contribution, @MQuy. Can you add the corresponding changes to the JRuby implementation?

@JasonLunn JasonLunn added jruby Issues unique to the JRuby interpreter json release notes: yes labels Jan 27, 2023
@MQuy MQuy force-pushed the feat/6178-ruby-enum-as-integer branch from 1163142 to 6348512 Compare February 2, 2023 03:50
@MQuy
Copy link
Contributor Author

MQuy commented Feb 2, 2023

hi @JasonLunn, I added JRuby implementation for emitting enums as integers. It looks like the pipeline won't run except it is triggered from your side?

@haberman haberman added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Feb 14, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Feb 14, 2023
@haberman
Copy link
Member

This PR looks good. Would you be able to rebase it on main? Sorry for the trouble, you've caught us in the middle of a migration to GitHub Actions.

@MQuy MQuy force-pushed the feat/6178-ruby-enum-as-integer branch 2 times, most recently from a49e8c4 to b87cc4e Compare February 14, 2023 19:15
@haberman haberman added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Feb 14, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Feb 14, 2023
@haberman
Copy link
Member

I'm seeing errors in the tests:

error: cannot find symbol
if (enumAsIntegers != null && printingEnumsAsInts.isTrue()) ***

@MQuy MQuy force-pushed the feat/6178-ruby-enum-as-integer branch from b87cc4e to fe471cc Compare February 15, 2023 07:59
@MQuy MQuy force-pushed the feat/6178-ruby-enum-as-integer branch from fe471cc to ee31aa3 Compare February 15, 2023 08:11
@quy-cbh
Copy link

quy-cbh commented Feb 15, 2023

I'm seeing errors in the tests:

error: cannot find symbol
if (enumAsIntegers != null && printingEnumsAsInts.isTrue()) ***

@haberman, my bad, updated

@haberman haberman added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Feb 16, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Feb 16, 2023
@MQuy
Copy link
Contributor Author

MQuy commented Mar 3, 2023

hi @haberman, @zhangskz and @JasonLunn, would love to have your updates on this feature.

@ericsalo
Copy link
Member

ericsalo commented Mar 7, 2023

I do apologize for having to ask this of you again but could you please rebase one more time? Things appear to be stuck even though the PR has been approved. Thanks.

@MQuy MQuy force-pushed the feat/6178-ruby-enum-as-integer branch from ee31aa3 to 90f986a Compare March 7, 2023 15:37
@MQuy
Copy link
Contributor Author

MQuy commented Mar 7, 2023

@ericsalo no worry, I rebased. Can you trigger pipelines again and merge the pull request. Thank you

@ericsalo ericsalo added the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Mar 7, 2023
@github-actions github-actions bot removed the 🅰️ safe for tests Mark a commit as safe to run presubmits over label Mar 7, 2023
@ericsalo ericsalo self-requested a review March 7, 2023 16:38
@copybara-service copybara-service bot closed this in 8aa2b17 Mar 7, 2023
@MQuy
Copy link
Contributor Author

MQuy commented Apr 18, 2023

hi @haberman, @zhangskz, @JasonLunn and @ericsalo is it possible that we include this feature in the next release?

@haberman
Copy link
Member

Since this was committed to the main branch, it will be present in the next release (4.23.x).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jruby Issues unique to the JRuby interpreter json ruby wait for user action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruby JSON serialize enums as ints
8 participants