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

Support String options #343

Merged
merged 4 commits into from
Jun 11, 2021
Merged

Support String options #343

merged 4 commits into from
Jun 11, 2021

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Jun 9, 2021

This PR

  • Allows using String as a command line option.
  • Fixes a bug that if the value of an option is invalid, we will still set the value.
  • Refactors our tests to properly clean up after each test.

@qinsoon qinsoon requested a review from caizixian June 9, 2021 01:41
@caizixian caizixian added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label Jun 9, 2021
Copy link
Member

@caizixian caizixian left a comment

Choose a reason for hiding this comment

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

LGTM

@caizixian caizixian merged commit dd2af22 into master Jun 11, 2021
@caizixian caizixian deleted the string-option branch June 11, 2021 05:35
@caizixian
Copy link
Member

caizixian commented Jun 11, 2021

I think this PR is sound. The issues in the JikesRVM binding test are likely due to the JikesRVM rebase, and @qinsoon is investigating. See also mmtk/mmtk-jikesrvm#70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants