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

Fixed the ID conversion issue for R4 #355

Merged
merged 5 commits into from
Oct 1, 2022
Merged

Fixed the ID conversion issue for R4 #355

merged 5 commits into from
Oct 1, 2022

Conversation

bashir2
Copy link
Collaborator

@bashir2 bashir2 commented Sep 30, 2022 •
edited
Loading

Description of what I changed

This fixes the ID conversion regression (Issue #55) that was reintroduced for R4 after PR #354.
The solution is a little bit hacky but works.

This is on top of #354, please only review the last commit.

Fixes #285

E2E test

TESTED:
Relied on current tests and uncommented a related unit-test that was commented out in #354.

Checklist: I completed these to help reviewers :)

  • I have read and will follow the review process.
  • I am familiar with Google Style Guides for the language I have coded in.
  • My IDE is configured to follow the Google code styles.
  • I have added tests to cover my changes. (If you refactored existing code that was well tested you do not have to add tests)
  • I ran mvn clean package right before creating this pull request and added all formatting changes to my commit.
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

bashir2
bashir2 commented Sep 30, 2022
View reviewed changes
Copy link
Collaborator Author

@bashir2 bashir2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review; PTAL

@bashir2 bashir2 mentioned this pull request Sep 30, 2022
Closed
omarismail94
omarismail94 approved these changes Sep 30, 2022
View reviewed changes
Copy link
Contributor

@omarismail94 omarismail94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one small nit

@bashir2 bashir2 merged commit 75a6c9b into google:master Oct 1, 2022
@bashir2 bashir2 deleted the R4-main-id branch October 4, 2022 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omarismail94 omarismail94 omarismail94 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add R4 support to Bunsen.
2 participants
@bashir2 @omarismail94