search

Wunderfitz / harbour-piepmatz

Piepmatz is a Twitter client for Sailfish OS
GNU General Public License v3.0
51 stars 17 forks source link

WIP: Pass Twitter client secrets as build arguments #107

Open Thaodan opened 3 years ago

Thaodan commented 3 years ago

Instead of having the user to create a header file. The build arguments can be set in qt creator project settings.

Wunderfitz commented 2 years ago

As I finally have some time to look at it, maybe some questions - partially already addressed in direct chat, some not. ;)

In general, some automation is definitely good - always wanted to setup an automated build. However, I'd like to avoid that the Twitter client ID and secret are visible to people who just take a look at the logs or the setup files. How would this work here? Moreover, some documentation for the README how to set up the build would be needed - especially as the current Build section would become outdated with this change.

Thaodan commented 2 years ago

The secrets would appear in the build logs but not anywhere else except the qt creator usersettings for this project which you don't upload.

I will update docs once you are ok with the functionality. Please also test how it works for you.

Wunderfitz commented 2 years ago

Mhmm, not really sure then. Using GitHub Actions with built-in secrets seems to work better, see e.g. the build contributed by @jgibbon for https://github.com/Wunderfitz/harbour-fernschreiber. From my perspective there is no urgent need to invest a lot of time here then. Feel free to do so, but the thing with the secrets would be important - also when it comes to additional integrations in the future where other secrets/credentials are needed...

Thaodan commented 2 years ago

This should be combined with those or we can generate the header add build time with secrets passed to qmake. The difference is that the build system knows about the secrets where in the other solution its hacked into the builder.

You don't provide build logs for fernschreiber either so I don't see the issue of having those in the log?

Wunderfitz commented 2 years ago

Of course, I don't provide the build log, probably wouldn't be that much beneficial... But if there is a public build, things would be different...

Thaodan commented 2 years ago

Then moving them to a generated header is sufficient.

Thaodan commented 2 years ago

I refactored the PR to use QMAKE_SUBSITUTION PR instead now this avoids having the credentails all over the build log. This works similar as in aboutdata.cpp.in in communi-sailfish: https://github.com/communi/communi-sailfish/blob/master/src/app/app.pro#L56