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

Add CD test #1068

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add CD test #1068

wants to merge 2 commits into from

Conversation

BPerlakiH
Copy link
Collaborator

@BPerlakiH BPerlakiH commented Jan 3, 2025

Fixes: #1067
Testing CD steps

@BPerlakiH BPerlakiH linked an issue Jan 3, 2025 that may be closed by this pull request
@BPerlakiH BPerlakiH marked this pull request as ready for review January 3, 2025 17:30
@BPerlakiH BPerlakiH requested review from rgaudin and kelson42 January 3, 2025 17:30
@codecov-commenter
Copy link

codecov-commenter commented Jan 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.87%. Comparing base (4a0d565) to head (2bb4a7d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1068      +/-   ##
==========================================
+ Coverage   36.86%   36.87%   +0.01%     
==========================================
  Files         127      127              
  Lines        7492     7492              
==========================================
+ Hits         2762     2763       +1     
+ Misses       4730     4729       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kelson42
Copy link
Contributor

kelson42 commented Jan 4, 2025

@rgaudin @BPerlakiH I'm not sure I want this is main. This shoudl be done adhoc in a PR when needed and then removed... or do I miss the point?

@rgaudin
Copy link
Member

rgaudin commented Jan 4, 2025

There are a couple of issues with this PR:

  • the goal is a bit fuzzy. What do we want to test exactly? Compilation? Notarization? Upload?
  • It's a duplicate of the current CD workflow. If this is meant to be ran adhoc on a test branch, that's fine (and we dont need a PR) but if this is going to be merged that's a duplicate of a long and complicated file.
  • it's expecting a manual trigger to run (workflow_dispatch) which is great for tests but needs to be on main branch. This is tied to previous point.

I believe what we want is the CD workflow to be able to run outside the release event on a different event (specific tag or workflow_dispatch) and in this case have conditions on some step (like the upload one).

@BPerlakiH
Copy link
Collaborator Author

@kelson42 @rgaudin At the moment, this is only temporary. Without merging it I don't have the option to run the CD build manually. I only see these options:
Screenshot 2025-01-05 at 12 38 04

and under CD, there's no manual run.
Since the original CD file is conditionally based on the trigger (either schedule, TestFlight or release), simply adding the workflow_dispatch (manual run) is not enough,
therefore I thought, that duplicating the file for test purposes makes more sense. Once the macOS FTP issue is solved, I am happy to delete this file.

@BPerlakiH
Copy link
Collaborator Author

Or maybe there's a way to run this without merging it, that would be even better, but I couldn't find it.

@kelson42
Copy link
Contributor

kelson42 commented Jan 5, 2025

Or maybe there's a way to run this without merging it, that would be even better, but I couldn't find it.

This is what @rgaudin said a few days ago: all worflowsmcan be run in PR, you just need to put the right event there to be triggered.

@BPerlakiH
Copy link
Collaborator Author

@kelson42 @rgaudin Maybe I am getting this wrong, and there's a way to do this... I am not sure.

By the first look, it seems to me that it's a security implementation from GitHub, so that unverified code (not merged) cannot modify the current workflows.

If you know a way how to run this without merging, I would be happy to learn that.

@kelson42
Copy link
Contributor

kelson42 commented Jan 6, 2025

I don't understand why fixing a CI/CD which got broken a few weeks ago generates all that fuzz. why you don't just fix it in a PR?

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.

Create test-bed for macOS FTP builds
4 participants