Closed gliptak closed 4 years ago
@ogarcia please review
The PR must be in develop not in master. Anyway, the gradle build is done before any push, I don't see the sense of this PR.
I see this was closed, but I did have a few comments:
It looks like this PR is set to automatically run test code anytime master is updated. That's pretty cool.
I don't think there are any automated tests in open sudoku. It might be good to have some, but I don't think the gradle test task would actually do anything. Maybe if we had tests this github action would make more sense?
This could be used to automatically build in the cloud everytime master is updated. Sure, master is locally verified, but building automatically has its merits too.
My two cents. :-)
On Tue, Feb 18, 2020, 9:00 AM Óscar García Amor notifications@github.com wrote:
Closed #63 https://github.com/ogarcia/opensudoku/pull/63.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ogarcia/opensudoku/pull/63?email_source=notifications&email_token=AOETRWC42XL3HJNPEQOPEFLRDQH2ZA5CNFSM4KWI6KHKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOWWWM3GI#event-3048000921, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOETRWHPYI54NDLPY2ZGDB3RDQH2ZANCNFSM4KWI6KHA .
@jlnordin currently this is configured to build all PRs (green checkmark) and the master when updated. While there no tests currently, compilation success is additional information which is not present for this project currently. After merging, we can update to publish a "release" APK on each master commit.
I quite like automatically building all PRs.
Oscar, thoughts?
On Tue, Feb 18, 2020, 9:51 AM Gábor Lipták notifications@github.com wrote:
@jlnordin https://github.com/jlnordin currently this is configured to build all PRs (green checkmark) and the master when updated. While there no tests currently, compilation success is additional information which is not present for this project currently. After merging, we can update to publish a "release" APK on each master commit.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ogarcia/opensudoku/pull/63?email_source=notifications&email_token=AOETRWHP3L6XIBK3TBOLOUTRDQNYZA5CNFSM4KWI6KHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMC6ODA#issuecomment-587589388, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOETRWBJLELW4OGZWRYKSNDRDQNYZANCNFSM4KWI6KHA .
The automagical builds can be done with any CD/CI, but the APK must be signed and, for security reasons, this cannot be done in a PR because not have control about code in PR and could be malicious.
If we had automated test then this have more sense.
I could set up an automatic build for every tag, but without unit tests I don't see the advantage because is a work that I'm doing already to upload the new versions to Play Store.
@ogarcia committers of this repo have control of what is pushed to the master
branch.
You could upload debug and/or release certs as secrets and build/release signed APKs (for master
branch only)
https://medium.com/better-programming/ci-cd-for-flutter-apps-using-github-actions-b833f8f7aac https://github.com/nabilnalakath/flutter-githubaction
@gliptak this is what I commented when say: I could set up an automatic build for every tag...
Signed-off-by: Gábor Lipták gliptak@gmail.com
https://github.com/gliptak/opensudoku/pull/1/checks?check_run_id=449613236