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

Move entitlements file to mac folder #2529

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Move entitlements file to mac folder #2529

merged 1 commit into from
Mar 29, 2022

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Mar 19, 2022 •
edited
Loading

Short description of changes
Moves the Jamulus.entitlements file to the mac/ folder to clean up the root folder

CHANGELOG: Internal: Moved entitlements file to mac/ folder to clean up the repo root.

Context: Fixes an issue?
No. Clean up only.

Does this change need documentation? What needs to be documented and how?
No.

Status of this Pull Request
Should be tested by @emlynmac if it still builds correctly. Also it reminds me of the sandbox writing issue (https://developer.apple.com/forums/thread/666634)

What is missing until this pull request can be merged?
Needs testing - especially with a signed build.

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@hoffie hoffie added this to the Release 3.9.0 milestone Mar 19, 2022
hoffie
hoffie approved these changes Mar 19, 2022
View reviewed changes
Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Code looks good, an actual signing test should be run (no signing has been tested after the autobuild refactoring either...).

@ann0see
Copy link
Member Author

ann0see commented Mar 20, 2022

@emlynmac coukd you please git checkout my repo and push a signed beta/nightly with these changes?

@ann0see ann0see mentioned this pull request Mar 22, 2022
Merged
5 tasks
@ann0see ann0see force-pushed the patch/moveEntitlements branch 2 times, most recently from b2df90d to 33dd49c Compare March 24, 2022 21:16
@ann0see ann0see force-pushed the patch/moveEntitlements branch 2 times, most recently from 0351261 to 75ccadc Compare March 24, 2022 21:20
@ann0see
Copy link
Member Author

ann0see commented Mar 27, 2022

Except for the infinitly long running builds on emlyn's fork, I think this is ok.

@emlynmac
Copy link
Contributor

Except for the infinitly long running builds on emlyn's fork, I think this is ok.

So to be clear, master branch no longer builds on my fork. It is consistently hanging on the build macOS stage, so whatever changes broke the signing step, it is not in this PR.

@ann0see
Copy link
Member Author

ann0see commented Mar 28, 2022

Yes. I know. I'll try to run CodeQl on the legacy build instead...

@hoffie
Copy link
Member

hoffie commented Mar 28, 2022

Yes. I know. I'll try to run CodeQl on the legacy build instead...

See chat -- I ran a non-CodeQL master build with signing on @emlynmac's branch and it worked: https://github.com/emlynmac/jamulus/actions/runs/2054387680

@ann0see ann0see requested a review from softins March 28, 2022 19:32
softins
softins approved these changes Mar 29, 2022
View reviewed changes
Copy link
Member

@softins softins left a comment

Choose a reason for hiding this comment

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

Happy to approve, assuming it builds correctly, but need to be careful about possible merge conflicts with #2563, due to the file moving.

@ann0see
Copy link
Member Author

ann0see commented Mar 29, 2022

@emlynmac I‘m going to merge this now. You‘ll probably need to fix conflicts

@ann0see ann0see merged commit bc5fa87 into jamulussoftware:master Mar 29, 2022
@ann0see ann0see deleted the patch/moveEntitlements branch March 29, 2022 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoffie hoffie hoffie approved these changes

@softins softins softins approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release 3.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ann0see @emlynmac @hoffie @softins