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

Automated build for Android APK (updated) #880

Merged
merged 23 commits into from
Jan 29, 2021

Conversation

nefarius2001
Copy link
Contributor

Credit and thanks to @ann0see for contributing & support

Resolves #835 Automated build for Android APK

open/related issues, that I see as upcomming improvements, but not part of this PR/issue:
#856 harmonized folders
#857 multiple APK-platforms

this PR creates automatically:

  • a prerelease for all tags that start with "r" and contain "beta"
  • a release for all tags that start with "r"
  • I ask you to accept the PR with this minimal behaviour and lets continue the discussion about behaviour in Automated github builds #854

The build process for android can be improved still, but I think it is already very good to have a working state as basis.

this replaces #878

@ann0see
Copy link
Member

ann0see commented Jan 23, 2021

@nefarius2001 see my comments from the last PR

@corrados
Copy link
Contributor

Is this ready to be merged now?

Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Probably soon. Just need to clarify a few things (see review.)

Note to corrados: as far as I understand, with these changes the latest tag is no longer automatically moved.

@nefarius2001
Copy link
Contributor Author

nefarius2001 commented Jan 24, 2021

Is this ready to be merged now?

#883 needs to be resolved as a part of this.

@ann0see
Copy link
Member

ann0see commented Jan 27, 2021

I will re-check if everything works. Afterwards, I think, this can be merged @nefarius2001 ?

@pljones
Copy link
Collaborator

pljones commented Jan 27, 2021

I will re-check if everything works. Afterwards, I think, this can be merged @nefarius2001 ?

Did anyone confirm if Code QL scans can or cannot be done on the Android build?

Also, @ann0see, it says you've got "Changes requested". Have they been made?

@ann0see
Copy link
Member

ann0see commented Jan 27, 2021

I didn't. I'm currently simulating a few things on my repo and have found a few "issues":

  1. If the latest tag is pushed, currently nothing happens (we have to add the "latest" tag to the .yml file)
  2. The Changelog detection is broken since the changelog still contains *git not *dev
  3. Currently checking: What does the latest release output? It's currently just labelled as pre-release which is not what I think it should be labelled.
  4. The flatpak is not named after the version of Jamulus (But I couldn't get it to run on my debian 10 machine)

For my proposed changes, see all differences until ann0see@66e0b6e



if pushed_name == "latest":
print('this reference is a Latest-Tag')
Copy link
Member

Choose a reason for hiding this comment

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

Why is the latest release a prerelease?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didnt care there, I thought the idea of this tag had died.

GitHub has a „latest release“ feature anyway.

Copy link
Member

Choose a reason for hiding this comment

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

I’d there an URL which is always accessible and pointing to the latest release? I think @corrados said somewhere that probably many people use this somehow. I also think that a url which always points to the latest release is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

look at this:
https://github.com/nefarius2001/jamulus/releases/latest

It points to the release with the youngest creation time, and is not related to the "latest" tag. I'd prefer that, since then the files also keep their version in name. But whoever actually does the releases, has the last word on how they should be done.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I didn't know about this feature.

since then the files also keep their version in name

I would like to have a steady link which we could link on the jamulus.io page as (second) mirror.
The user would then click on "Download Jamulus" and the download would then start right away without leaving jamulus.io

This would provide a better user experience since they wouldn't be redirected to sourceforge which some people dislike (there are issues on that here).

The binaries would still be uploaded to SourceForge and maybe even linked on jamulus.io so that we have multiple mirrors. Multiple mirrors provide redundancy and also higher availability. If one is down, there is still the other one.

@ann0see
Copy link
Member

ann0see commented Jan 27, 2021

it says you've got "Changes requested". Have they been made?

I requested changes: #880 (comment)

#880 (comment)

etc. So it's not yet ready to be merged. They must be discussed before a merge.

@ann0see ann0see self-assigned this Jan 27, 2021
@ann0see
Copy link
Member

ann0see commented Jan 27, 2021

Did anyone confirm if Code QL scans can or cannot be done on the Android build?

@nefarius2001 is/was working on something (according to GH Actions on his repo), but IDK if that's related to CodeQl.

- "r*" # run this action if a tag beginning with r is created
workflow_dispatch:

- "r*"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- "r*"
- "r*"
- "latest"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added latest as trigger
decide yourself if you want to use the "latest" tag or leave that code unused/triggered

@nefarius2001
Copy link
Contributor Author

nefarius2001 commented Jan 27, 2021

Hi,
sorry for the late reply.

I am still working on the overall build, packing CodeQL and artifacts in one job. ( #854 and #856 )
Do you prefer to that as a separate PR then, or pe a part of "this"?
Checking CodeQL for Android is still on my list for later :)

@pljones
Copy link
Collaborator

pljones commented Jan 27, 2021

Checking CodeQL for Android is still on my list for later :)

Best to raise the issue for tracking it :)

@nefarius2001
Copy link
Contributor Author

nefarius2001 commented Jan 28, 2021

changelog was not working, fix in progress

so status is: not ready :(

@nefarius2001
Copy link
Contributor Author

looks good to me
https://github.com/nefarius2001/jamulus/releases/tag/r_test
https://github.com/nefarius2001/jamulus/releases/tag/r_test_beta

ready to merge from my side

Resolves:
#835 Automated build for Android APK

still in progress, but independent:
#856 harmonized folders

  • one job for artifacts & codeql

@corrados
Copy link
Contributor

@ann0see Are you fine with merging it now?

Copy link
Member

@ann0see ann0see left a comment

Choose a reason for hiding this comment

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

Yes.

@corrados corrados merged commit 02d0785 into jamulussoftware:master Jan 29, 2021
@pljones pljones added this to the Release 3.7.0 milestone Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

Successfully merging this pull request may close these issues.

Automated build for Android APK
4 participants