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

ci: build pull requests #1228

Merged
merged 40 commits into from
Sep 20, 2023

Conversation

BenjaminHalko
Copy link
Member

@BenjaminHalko BenjaminHalko commented Sep 2, 2023

This should hopefully allow compose-dev pr's to build the manager. (Though I haven't been able to test it much)

This is a combination of flutter pr build and compose release.

This workflow is not for building & uploading to GitHub releases, it is meant for building/verifying compose pull requests.

@BenjaminHalko BenjaminHalko added the ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager label Sep 2, 2023
@Ushie Ushie changed the base branch from main to dev September 2, 2023 22:29
@Ushie Ushie changed the title actions: Support compose pr ci(debug-builds): add workflow for compose-dev branch Sep 2, 2023
Copy link
Member

@validcube validcube left a comment

Choose a reason for hiding this comment

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

I have no objections, would anyone from team like to comment on this?

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Sep 3, 2023

This duplicates an existing workflow in the wrong branch. The PR target should be the compose branch and it should replace an existing release workflow if there is any

@oSumAtrIX oSumAtrIX changed the base branch from dev to compose-dev September 3, 2023 16:16
@oSumAtrIX oSumAtrIX changed the title ci(debug-builds): add workflow for compose-dev branch ci: add add release workflow Sep 3, 2023
@Ushie
Copy link
Member

Ushie commented Sep 3, 2023

The workflow will not run if it doesn't reach the main (dev), it can't simply stay in the compose-dev branch

@BenjaminHalko
Copy link
Member Author

BenjaminHalko commented Sep 3, 2023

This workflow is meant to mimic the pr-build workflow for Flutter / the release workflow (without the push trigger) that was for compose on the previous repo and should be merged with main so that it triggers on pull requests for compose so that it can verify that the pr compiles / can be easily downloadable for testing purposes.

#1235 is the potential release workflow for compose that will stay on compose-dev until compose-dev is merged with main.

@BenjaminHalko BenjaminHalko changed the base branch from compose-dev to dev September 3, 2023 17:13
@BenjaminHalko BenjaminHalko changed the title ci: add add release workflow ci(debug-builds): add workflow for compose-dev branch Sep 3, 2023
@BenjaminHalko BenjaminHalko changed the title ci(debug-builds): add workflow for compose-dev branch ci(debug-builds): add workflow for compose-dev pull requests Sep 3, 2023
@BenjaminHalko BenjaminHalko changed the title ci(debug-builds): add workflow for compose-dev pull requests ci(compose-pr-build): add workflow for compose-dev pull requests Sep 3, 2023
@oSumAtrIX
Copy link
Member

oSumAtrIX commented Sep 3, 2023

Apparently, the workflow should also run if not on the main or dev branch, so it should be introduced with the compose branch if it is meant to create compose releases.

@BenjaminHalko
Copy link
Member Author

@oSumAtrIX If I were to switch the branch to compose-dev would I need to create a separate pull request? As this on targets dev and switching the branch inside the pr seems to create conflicts (unless I am just doing it wrong).

@oSumAtrIX
Copy link
Member

The conflicts happen because your source branch is based on the current dev branch. If you simply cherry pick your commits on top of the compose branch and force push it to the PRs source branch, this should solve the merge conflicts when you change the target to the compose branch

@oSumAtrIX
Copy link
Member

Currently, there is a workflow present in the compose-dev branch, but it does not run @PalmDevs may know more about this.

@Ushie
Copy link
Member

Ushie commented Sep 3, 2023

Apparently, the workflow should also run if not on the main or dev branch, so it should be introduced with the compose branch if it is meant to create compose releases.

It simply uploads the build as an artifact in Actions, it doesn't create a release

@oSumAtrIX
Copy link
Member

What do you mean by "it"? The workflow introduced by this PR or the existing one in the compose dev branch?

@Ushie
Copy link
Member

Ushie commented Sep 3, 2023

The one introduced by this PR

@oSumAtrIX
Copy link
Member

Sounds good, changing the target branch should be enough then.

@BenjaminHalko BenjaminHalko changed the base branch from dev to compose-dev September 3, 2023 17:46
@BenjaminHalko
Copy link
Member Author

@oSumAtrIX done

@Ushie

This comment was marked as outdated.

@BenjaminHalko
Copy link
Member Author

@Ushie I did some tests and for pull request workflows, it seems like the workflow has to be in the branch getting merged.

Example:

  • branch a has the workflow pr-build.yml.
  • branch b has the workflow pr-build-b.yml
  • if the pr targets branch a, pr-build.yml will trigger
  • if the pr targets branch b, pr-build-b.yml will trigger
  • if the pr targets branch c (which has no pull request workflows), nothing will happen.

@Ushie
Copy link
Member

Ushie commented Sep 3, 2023

It seems like what I said wasn't true, I'm not sure why I was strongly under the assumption that workflows are only recognised in the default branch, my comments can be ignored

@BenjaminHalko
Copy link
Member Author

@Ushie No worries!
@oSumAtrIX Is this ready to merge now?

@oSumAtrIX oSumAtrIX changed the title ci(compose-pr-build): add workflow for compose-dev pull requests ci: add workflow for compose-dev pull requests Sep 3, 2023
@BenjaminHalko
Copy link
Member Author

BenjaminHalko commented Sep 15, 2023

Like this?

path: |
            ${{ runner.home }}/.gradle/caches
            ${{ runner.home }}/.gradle/wrapper
            .gradle
          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }}

@BenjaminHalko
Copy link
Member Author

BenjaminHalko commented Sep 15, 2023

should it also be hashing other gradle files like settings.gradle.kts and build.gradle.kts?

@BenjaminHalko
Copy link
Member Author

Like this:

path: |
            ${{ runner.home }}/.gradle/caches
            ${{ runner.home }}/.gradle/wrapper
            .gradle
          key: ${{ runner.os }}-gradle-${{ hashFiles('**/*.gradle*', '*.gradle*') }}

@Ushie
Copy link
Member

Ushie commented Sep 15, 2023

gradle-wrapper.properties should be kept, I'm not sure if other files are necessary

@BenjaminHalko
Copy link
Member Author

sure, I just found that we do have a gradle-wrapper file. My bad.

@BenjaminHalko
Copy link
Member Author

@oSumAtrIX Is this ready for merge now?

.github/workflows/pr-build.yml Outdated Show resolved Hide resolved
.github/workflows/pr-build.yml Outdated Show resolved Hide resolved
.github/workflows/pr-build.yml Outdated Show resolved Hide resolved
@BenjaminHalko
Copy link
Member Author

@oSumAtrIX Is this ready to merge?

@oSumAtrIX
Copy link
Member

You got the greens from me ✅
image

@BenjaminHalko
Copy link
Member Author

@Ushie Anything else to add?

app/build.gradle.kts Outdated Show resolved Hide resolved
@BenjaminHalko
Copy link
Member Author

@Ushie done

@BenjaminHalko BenjaminHalko merged commit c4b9bb1 into ReVanced:compose-dev Sep 20, 2023
1 check passed
@BenjaminHalko BenjaminHalko deleted the support-compose-pr branch September 20, 2023 17:49
BenjaminHalko added a commit that referenced this pull request Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants