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

#2615 Process.euid= should accept String #2651

Closed
wants to merge 0 commits into from
Closed

#2615 Process.euid= should accept String #2651

wants to merge 0 commits into from

Conversation

ngtban
Copy link
Contributor

@ngtban ngtban commented May 1, 2022

@ngtban ngtban changed the title #2615 process euid should accept string #2615 Process.euid= should accept string May 1, 2022
@graalvmbot
Copy link
Collaborator

Hello Ban Nguy, thanks for contributing a PR to our project!

We use the Oracle Contributor Agreement to make the copyright of contributions clear. We don't have a record of you having signed this yet, based on your email address ngtban -(at)- gmail -(dot)- com. You can sign it at that link.

If you think you've already signed it, please comment below and we'll check.

@ngtban ngtban force-pushed the 2615-process-euid-should-accept-string branch from 9ebfc8e to 5642ec7 Compare May 1, 2022 14:26
@ngtban ngtban changed the title #2615 Process.euid= should accept string #2615 Process.euid= should accept String May 1, 2022
eregon
eregon reviewed May 3, 2022
View reviewed changes
src/main/ruby/truffleruby/core/process.rb Outdated Show resolved Hide resolved
spec/truffle/process/euid_spec.rb Outdated Show resolved Hide resolved
spec/truffle/process/euid_spec.rb Outdated Show resolved Hide resolved
spec/truffle/process/euid_spec.rb Outdated Show resolved Hide resolved
@ngtban ngtban requested a review from eregon May 3, 2022 12:57
@ngtban ngtban requested a review from eregon May 3, 2022 18:47
@ngtban
Copy link
Contributor Author

ngtban commented May 5, 2022

Hi @eregon , could you please help me have a look at the PR again? Thank you!

eregon
eregon approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@eregon eregon left a comment

Choose a reason for hiding this comment

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

Thanks!

@eregon eregon added the in-ci The PR is being tested in CI. Do not push new commits. label May 10, 2022
@eregon
Copy link
Member

eregon commented May 10, 2022

This is too many commits for the number of lines changed and the back-and-forth, so I'll squash them into 1 commit.

@eregon eregon force-pushed the 2615-process-euid-should-accept-string branch from bec7f93 to 345b7a5 Compare May 10, 2022 17:09
@eregon eregon added this to the 22.2.0 milestone May 10, 2022
graalvmbot pushed a commit that referenced this pull request May 11, 2022
@eregon
[GR-18163] Process.euid= should accept String (#2651)
5cedd64 
PullRequest: truffleruby/3344
@eregon eregon closed this May 11, 2022
@eregon eregon force-pushed the 2615-process-euid-should-accept-string branch from 047e7c2 to 92636d8 Compare May 11, 2022 13:34
@eregon
Copy link
Member

eregon commented May 11, 2022

This is merged in 5cedd64.
It's not marked in the GitHub UI as such because you used the resolve conflicts button on GitHub, but that shouldn't be used, see https://github.com/oracle/truffleruby/blob/master/CONTRIBUTING.md#changelog
So all is good, but please don't use that resolve conflicts button.

If you'd like, you could make a similar PR for Process.egid=, as mentioned in #2615 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eregon eregon eregon approved these changes

Assignees
No one assigned
Labels
in-ci The PR is being tested in CI. Do not push new commits. oca-signed
Projects
None yet
Milestone
22.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@ngtban @graalvmbot @eregon