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

Relax ProcessMonitor checks for execute bits #2717

Merged
merged 1 commit into from
May 26, 2018

Conversation

cararemixed
Copy link

This is part one of addressing issue #2714. We avoid breaking compatibility with the API in this change.

We should rely on the system to do these checks properly. Since we don't currently have a clean way to signal failure of execv, we'll assert that the path is a proper file first.

@SeanTAllen
Copy link
Member

@strmpnk the commit history on this is messed up

This is part one of addressing issue ponylang#2714. We avoid
breaking compatibility with the API in this change.

We should rely on the system to do these checks properly.
Since we don't currently have a clean way to signal failure
of execv, we'll assert that the path is a proper file
first.
@cararemixed
Copy link
Author

I think I had a tagged checked out before doing this and forgot to reset my branch point to something sane. It should be fixed now.

@SeanTAllen
Copy link
Member

@strmpnk should this go in changelog? if yes, in "Fixed"? its not a breaking change so i'd like to avoid "changed"

@cararemixed
Copy link
Author

This first part is non-breaking. So I'd say fixed makes sense. I'm new to the changelog policy here but should I update this PR with an entry?

@SeanTAllen
Copy link
Member

@strmpnk you can or for most we use the tags. which i just added. we have a bot that will add the correct changelog entry for you. same as wallaroo labs (ponylang is where wl took the code from)

@SeanTAllen SeanTAllen added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label May 25, 2018
@SeanTAllen
Copy link
Member

Excellent first commit @strmpnk. Thanks! and welcome to the contributors list.

@SeanTAllen SeanTAllen merged commit c65534f into ponylang:master May 26, 2018
ponylang-main added a commit that referenced this pull request May 26, 2018
dipinhora pushed a commit to dipinhora/ponyc that referenced this pull request Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants