search

Gericom / FastVideoDSPlayer

Player for the FastVideoDS format
29 stars 8 forks source link

Add a GitHub Actions workflow #2

Closed lifehackerhansol closed 2 years ago

lifehackerhansol commented 2 years ago

Also fix funny capitalization issue that broke compilation on Linux.

lifehackerhansol commented 2 years ago

This currently uploads with the file name FastVideoDSPlayer.nds, since that is what the makefile spits out.

However, I noticed in your pre-release, that you are using the name FastVideoDS.nds? Should I change the uploaded file name to that?

Gericom commented 2 years ago

It's because it uses the name of the folder in the Makefile for the name of the nds file. Seeing as twilight uses FastVideoDS.nds as name, it may be better to force that. By changing the Makefile to always just use that name.

Edit: Btw, with upload I assume you just mean a build artifact, not a github release?

lifehackerhansol commented 2 years ago

It's because it uses the name of the folder in the Makefile for the name of the nds file. Seeing as twilight uses FastVideoDS.nds as name, it may be better to force that. By changing the Makefile to always just use that name.

Edit: Btw, with upload I assume you just mean a build artifact, not a github release?

There is both a workflow for release and nightly. Nightly is just a build artifact, but release pushes the build artifact to the created release page automatically. (Hence no manual uploads etc). I can remove that if you don't want to.

I'll do the makefile changes.

Gericom commented 2 years ago

It is fine as long as it does not automatically push releases like in the GBARunner2 repo. I want to control that by myself.

lifehackerhansol commented 2 years ago

You will need to create the release yourself. When GitHub sees that, it'll build the app and upload it to the release you created.

Otherwise, on every commit there will be a build artifact on the Actions tab. Definitely not how GBARunner2 does it, don't worry. It moreso aligns with TWiLight's workflow only there's only one NDS file instead of a whole package.

In any case TARGET is now FastVideoDS.nds.

Gericom commented 2 years ago

Ah okay, that seems alright to me. Is it ready to merge then?

lifehackerhansol commented 2 years ago

Yeah, it's ready.

Gericom commented 2 years ago

Done! Seems to work!

Gericom commented 2 years ago

Btw, I used rebase now, but do you think it would normally be better to use merge?

lifehackerhansol commented 2 years ago

I personally avoid merge commits with a passion. This is a small project and isn't really necessary, it's not like the Linux kernel.

I usually rebase if there is not many commits, if there's a lot of commits in the PR (some people aren't too smart with Git) then I squash merge instead.

Gericom commented 2 years ago

Ah okay. Because I noticed it shows that I committed and you authored or so.