The GitHub actions build runner fails on PRs

[Olf0]

@ruditimmer, the GitHub actions build runner you implemented so nicely fails on Pull Requests (PRs), while it works fine once a PR has been merged (i.e., on exactly the same code). This pattern has become obvious throughout the last couple of PRs, see https://github.com/CepiPerez/filecase-sailfish/actions

Because it is a irritating when a correct PR fails and this counteracts the very purpose of a build pipeline (do detect build errors early), I would appreciate very much if you fix that, so the "build runner runs properly" / the "build workflow flows nicely". :wink:

Thank you!

[ghost]

I have made a PR to see if it would be fixed now. let's see

[Olf0]

I have made a PR to see if it would be fixed now. let's see

For reference: This is PR #18.

[ghost]

For now it works i need only to look to upload the build to release would do that tomorow.

[Olf0]

@ruditimmer, as a side note: The is something severely wrong with your git workflow. You are repeatedly pushing commits which were already merged, which messes up the commit history and will make reverting commits and "cherry picking" commits (between branches) hard to logistically impossible:

1. You PR #20 contains two older, already merged commits from me, see [1a] and [2a]. These were originally merged by Matias in [1] and [2].
2. You seem to be re-committing your own commits again, resulting in PRs with no changes, but re-added commit content. See e.g. PR #22, which has no changes, but two commits. Obviously the content of these commits was already there. In a way this is the opposite of point 1, where you re-committed my already merged commits as yours. I wonder how to achieve that without additional changes, but this must be avoided.
3. Please squash-merge your commits when merging a PR here, which results in a single upstream commit of all changes in your PR.
4. Please test your changes in your own repository at GitHub first, before merging them into the upstream repository (i.e., here) and then fixing them multiple times. You should have sufficient reason to assume that your changes are working, before committing them upstream.
5. Please use descriptive commit comments, i.e., more than just "Fix", "Final fix" etc.!

My strong suggestion is to use the GitHub web-frontend for handling PRs and merges from your GitHub repository to this upstream repository. Then you can let your local repository (on your computer) and your repository at GitHub interact in any way you like. But only create PRs and push them into this upstream repository from your own GitHub repository, when they are really ready. Plus use the GitHub's web-frontend for this, because it is quite fail-safe.

@CepiPerez, while I accepted your invitation as a contributor, I do think it is a bad idea to let three different people with vastly different styles of managing a repository directly commit into your upstream repository. Three people do not automatically form a team, if there is no coherence between them.

At least there should be a compulsory review of another contributor (for quality assurance), which can be configured in the GitHub repository settings.

But ultimately you should ask yourself what you want to achieve? I already suggested to meet a clear decision, if you want to stay the maintainer and be responsible for your six source code repositories at GitHub (but then you shall be the reviewer and handle this responsibly, i.e., really do review PRs), or if you want to get rid of this workload: Then you shall archive all six of your repositories (which is easily revert-able, BTW). See details how to achieve that.

[ghost]

Hi Olf,

Oh that's indeed bad sorry i dont see that is happend !

Ok i quit with it again sorry for that i'm not good with git stuff i use SmartGit here so it should be that software what is doing wrong. again sorry for that. I try it on my own repo but it seems Smartgit was doing something strange indeed. About description i indeed add a more explaine description but as told it seems SmartGit messed it up into some way.

So i will not commit anymore and close the case here. And will remove myself from the repo also as managing it directly.

[Olf0]

Oh that's indeed bad sorry i dont see that is happend !

Well, shit happens. The crucial point always is how to handle that. I.e., trying to handle mishaps constructively.

Ok i quit with it again sorry for that i'm not good with git stuff i use SmartGit here so it should be that software what is doing wrong. again sorry for that. I try it on my own repo but it seems Smartgit was doing something strange indeed.

Hey Rudi, no reason to give up that quickly. And I did not mean to be overly harsh, I just intended to concisely name the issues I perceived.

I tried to show you a way to avoid such mishaps: Prepare and test everything at your own GitHub repository. When it is really ready and working, then solely use the GitHub web-frontend to create Pull Requests from your to this repository. The GitHub web-frontend is really nice, easy and fail-safe.

About description i indeed add a more explaine description

Yes, that would be nice in order to understand what was altered without having to read the source code changes in detail.

So i will not commit anymore and close the case here. And will remove myself from the repo also as managing it directly.

Please do not do that: Both, cease committing and removing yourself as a contributor! If Cepi switches on mandatory reviews as I suggested, then three people is the minimal number for that to work well in practice.

[ruditimmermans]

Hi Olf0,

The build script is now fixed i have done a lot a of testing on my repo before i do a merge request. I fix also the SD-Card show now it show this before it was broke.

Greetings, Rudi

[Olf0]

Hello Rudi, nice to hear from you.

Thank you very much for your contributions, especially switching to the build infrastructure by CODeRUS for the GitHub-actions runs here. This looks much better (i.e., clearer & cleaner) and easier to maintain now. Actually I considered dropping the GitHub-actions based builds, because I tried to trigger handling FileCase as an "abandoned app" at SailfishOS:Chum (so it is build there) and the original build framework did not appear to be in a good shape. But it is much nicer to have a first build check in one's own repository, I trust coderus' build infrastructure, and currently there also is no progress at SailfishOS:Chum.

I reviewed your Pull Requests #23 and made some small changes; thank you for leaving "allow commits from maintainers" on, so I was able to do that. For details, see my notes there.

A single general remark: Please try to cover a single (i.e., just one) topic per PR, not multiple topics in one PR. Taking PR #23 as an example, PR #24 would have belong to/in it (I already added that before reviewing #24), but commits 0ed2aaa, 572e3ef, 5d2f25b, 9c693fe, 3c35644, aa1e173 plus my commit 5e084f2 should have been placed into a separate PR "Fix show SD-Card". Never mind, this is O.K. for now, and I also regularly forget to clearly separate different topics into different PRs.

[Olf0]

Well, PR #23 did not fix anything, I already wondered how it could when reviewing it for merging, but was assured that it would be working. The PRs #6, #8, #18, #19, #20, #22 and #23, which all deal with establishing a working, GH-actions based build runner seem to be written in a trial-and-error approach without having understood the documentation and examples.

It took me a couple of hours yesterday to read the documentation and examples, plus to test a few things, in order to get the build runner properly working on every created tag or commit and to identify the unrelated files, which were added in the belief to "heal" the build process. This resulted in PR #25.