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

Build: Enable GitHub Actions based CI #55

Merged
merged 10 commits into from
Sep 13, 2023
Merged

Conversation

Johni0702
Copy link
Collaborator

@Johni0702 Johni0702 commented Sep 11, 2023

Also removes 1.15 because of a bug in archloom (1).
Diff between 1.16.2-forge before and after the removal shows no significant
changes:

diff -r org/main/java/gg/essential/universal/UGraphics.java versions/1.16.2-forge/build/preprocessed/main/java/gg/essential/universal/UGraphics.java
55,58c55,58
< //$$ import net.minecraft.client.renderer.Matrix3f;
< //$$ import net.minecraft.client.renderer.Matrix4f;
< //$$ import net.minecraft.client.renderer.Vector3f;
< //$$ import net.minecraft.client.renderer.Vector4f;
---
> //$$ import org.lwjgl.util.vector.Matrix3f;
> //$$ import org.lwjgl.util.vector.Matrix4f;
> //$$ import org.lwjgl.util.vector.Vector3f;
> //$$ import org.lwjgl.util.vector.Vector4f;
diff -r org/main/kotlin/gg/essential/universal/vertex/VanillaVertexConsumer.kt versions/1.16.2-forge/build/preprocessed/main/kotlin/gg/essential/universal/vertex/VanillaVertexConsumer.kt
9,12c9,12
< //$$ import net.minecraft.client.renderer.Matrix3f
< //$$ import net.minecraft.client.renderer.Matrix4f
< //$$ import net.minecraft.client.renderer.Vector3f
< //$$ import net.minecraft.client.renderer.Vector4f
---
> //$$ import org.lwjgl.util.vector.Matrix3f
> //$$ import org.lwjgl.util.vector.Matrix4f
> //$$ import org.lwjgl.util.vector.Vector3f
> //$$ import org.lwjgl.util.vector.Vector4f
diff -r org/main/kotlin/gg/essential/universal/wrappers/message/UTextComponent.kt versions/1.16.2-forge/build/preprocessed/main/kotlin/gg/essential/universal/wrappers/message/UTextComponent.kt
182c182
<         //$$     is String -> StringTextComponent(hoverValue as String)
---
>         //$$     is String -> TextComponentString(hoverValue as String)
185c185
<         //$$     else -> StringTextComponent(hoverValue.toString())
---
>         //$$     else -> TextComponentString(hoverValue.toString())

To be squashed on merge.

Publish doesn't work yet due to secrets not being set up, but otherwise should be good to review.

@Johni0702 Johni0702 force-pushed the feature/github-actions branch 4 times, most recently from a9906a7 to 6233535 Compare September 11, 2023 14:03
Also removes 1.15 because of a bug in archloom ([1]).
Diff between 1.16.2-forge before and after the removal shows no significant
changes.

[1]: architectury/architectury-loom#151
Copy link
Member

@DJtheRedstoner DJtheRedstoner left a comment

Choose a reason for hiding this comment

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

The reason this caused a maven publish with the branch name HEAD is that pull_request triggers are based on the merge commit for the PR, so the branch name isn't associated. We'll probably need some override (env variable?) in EGT that actions can pass through from github.event.pull_request.head.ref.

key: gradle-wrapper-${{ hashFiles('**/gradle-wrapper.properties') }}
- uses: actions/cache@v3
with:
path: ~/.gradle/caches
Copy link
Member

Choose a reason for hiding this comment

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

We should also cache the loom-cache folders in the project, this contains remapped mods and MC versions with AW/ATs applied (only the first is relevant to our mods though). See Sk1erLLC/Patcher#72

@Johni0702
Copy link
Collaborator Author

The reason this caused a maven publish with the branch name HEAD is that pull_request triggers are based on the merge commit for the PR, so the branch name isn't associated. We'll probably need some override (env variable?) in EGT that actions can pass through from github.event.pull_request.head.ref.

Hm, looks like that's the case. I expected it to use something like refs/pull/55/merge cause that's what it fetches and checks out, but I suppose checking out an arbitrary ref is more like checking out a specific commit than a branch, and git rev-parse can't know you did it.
Luckily we already have an override in EGT because the old CI used one all the times.

@Johni0702 Johni0702 merged commit fd989a7 into master Sep 13, 2023
1 check passed
@Johni0702 Johni0702 deleted the feature/github-actions branch September 13, 2023 11:56
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.

2 participants