SimonHalvdansson / Harmonic-HN

Modern Android client for Hacker News
https://play.google.com/store/apps/details?id=com.simon.harmonichackernews
Apache License 2.0
620 stars 39 forks source link

Fix workflows target APK path (#72) #74

Closed streambinder closed 1 year ago

AppearamidGuy commented 1 year ago

I'm not familiar with how Obtainium works, but it looks like it doesn't sign apks for you. So this won't work, because build creates unsigned apk that can't be installed.

Also I disagree with changes to build.yaml, especially the rename. The old name was descriptive and reflected the purpose of the action, the new name doesn't do any of that. Also the new name is just wrong, because the action can be triggered manually in addition to opening pr/push to main.

streambinder commented 1 year ago

So this won't work, because build creates unsigned apk that can't be installed.

It does work, it just does not ship with any signing from the original author. Even if that would appreciated from the author to do, that's neither something necessary nor something I can do for him. It's pretty straightforward work, once a repository secret with the signing key is added; as straightforward as adding a further step in the build job. It's just not something I can do for him.

The old name was descriptive and reflected the purpose of the action.

IMHO it's just redundant: in case name is not passed, the action uses field (i.e. action name) is used instead. And that's already pretty clear. But that's just my opinion.

Also the new name is just wrong.

Workflow is triggered primarily on push, regardless it's done directly against main or in a PR. And as for workflow_dispatch it's a minor use-case, visibly not used as there's no such case in events history for that workflow.

To conclude, IMHO, every claim here just pretends to be absolute and justified by specious arguments, whilst it only is a very questionable questionable personal point of view. Even more considering this is a PR already approved by the maintainer of the project which primarily introduced, along with #70, automatic releasing of assets on tagging, which is a feature nobody but me asked for. People in opensource projects should be just thankful for others to be taking time to contribute freely on something, for free, and that gratitude should be shown via technically valid points formulated in a respectful manner. Don't get me wrong, you could open a PR for all points you mentioned and get it merged straightaway, and I couldn't care less; my point is just don't be rude and build a technically valid and constructive discussion rather.

AppearamidGuy commented 1 year ago

So this won't work, because build creates unsigned apk that can't be installed.

It does work, it just does not ship with any signing from the original author.

To test whether my argument was specious or not (thanks for teaching me this fancy word btw) I've created a test repo with an exact copy of your action. You can find it here: https://github.com/AppearamidGuy/unsigned-obtainium-test As expected, when I tried to install the app via Obtainium it gave me an error:

Some Errors Occured
Invalid: [com.example.unsignedobtainiumtest]

[...] gratitude should be shown via technically valid points [...]

Couldn't agree more!

[...] in a respectful manner.

Ok, I'll work on my communication skills

streambinder commented 1 year ago

thanks for teaching me this fancy word

Always glad to help :)

To test whether my argument was specious or not I've created a test repo with an exact copy of your action. You can find it here: https://github.com/AppearamidGuy/unsigned-obtainium-test. As expected, when I tried to install the app via Obtainium it gave me an error.

Are you just telling me that your claims were not pretentious because you — only afterwards — tested them? Or that my contribution in getting Harmonic to release on GitHub Releases as well as getting some CI in place to make it automatic on tags creation for better integration and workability with git tooling is arguable because Obtainium — which, as far as I know, I’m the only user of, Harmonic-wise —, given it’s relying on Android APIs for application installation, cannot install an unsigned application? And all this, when there was no GitHub Release in the first place before that? And more importantly, when I’m not the one who can fix such case, given that I own no signing key for Harmonic?

I honestly don’t see how this all can have any impact on you already, as well as how you could notice any misbehaviour in all these changes, given that you’re neither GitHub Release user for Harmonic (there weren't any before raising the issue), nor a Obtainium user (there wasn't any that had raised the issue, at least) and, more importantly, that this hasn’t caused any regression, but only partially working — I’ll agree on that — new feature which I, as Obtainium user, would have noticed as soon as @SimonHalvdansson would have pushed a new tag.

Thanks for taking your free time to prove couple of days in advance that we still need some other doing before reaching what I originally desired, but I still can’t see your point in bringing up all this.