Skip to content

set executable flag on mac as well - #65

Merged
chenkasirer merged 1 commit into
mainfrom
darwin
Oct 16, 2025
Merged

chenkasirer merged 1 commit into
mainfrom
darwin

Conversation

@chenkasirer

Copy link
Copy Markdown
Member

Generating proto files fails on the github action mac machine, I suspect it's due to not setting the binary as executable.

Copilot AI review requested due to automatic review settings October 16, 2025 12:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a permissions issue that prevented the protoc binary from executing on macOS in GitHub Actions. The executable flag was previously only set on Linux systems, but macOS (Darwin) also requires this permission.

Key Changes:

  • Extended platform check to include macOS when setting executable permissions on the protoc binary

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/compas_pb/invocations.py Modified platform check to include "Darwin" (macOS) alongside "Linux" for setting executable flag
CHANGELOG.md Added entry documenting the fix for protoc permission issues on Mac

Comment thread CHANGELOG.md
@@ -22,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

Copilot AI Oct 16, 2025

Copy link

Choose a reason for hiding this comment

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

The changelog entry is placed under the '### Changed' section, but fixes should be documented under a '### Fixed' section according to Keep a Changelog conventions.

Suggested change
### Fixed

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

do you see a Fixed section?

@WeiTing1991 WeiTing1991 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@chenkasirer
chenkasirer merged commit 459b82e into main Oct 16, 2025
18 checks passed
@chenkasirer
chenkasirer deleted the darwin branch October 16, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants