-
Notifications
You must be signed in to change notification settings - Fork 11
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
Snap build configuration #92
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #92 +/- ##
=======================================
Coverage 14.42% 14.42%
=======================================
Files 10 10
Lines 520 520
=======================================
Hits 75 75
Misses 429 429
Partials 16 16 Continue to review full report at Codecov.
|
Two general additions:
|
We talked in chat a bit, but just to update the report.
On Sep 19 2021, at 3:26 pm, Lukas Malkmus ***@***.***> wrote:
We use GoReleaser ***@***.***/0?redirect=https%3A%2F%2Fgoreleaser.com&recipient=cmVwbHkrQUFSUVVIUDY3STc3STY1Q003TE5UVE43S043QUxFVkJOSEhEWEZNQkVNQHJlcGx5LmdpdGh1Yi5jb20%3D) to release Axiom CLI. You might want to take a look on how to integrate: https://goreleaser.com/customization/snapcraft ***@***.***/1?redirect=https%3A%2F%2Fgoreleaser.com%2Fcustomization%2Fsnapcraft&recipient=cmVwbHkrQUFSUVVIUDY3STc3STY1Q003TE5UVE43S043QUxFVkJOSEhEWEZNQkVNQHJlcGx5LmdpdGh1Yi5jb20%3D). We can pair on this during the week and figure it out.
To work with the Canonical build system we need snapcraft to be the root tool, so using goreleaser like that won't really work.
I don't know if that helps: You can completely omit the cli config file if you want and configure by environment or flags only. Check axiom help environment and the README of axiom-go that powers cli. There is also a configuration difference when using Cloud and Selfhost: AXIOM_ORG_ID (or the corresponding config file value/command line flag) must be set for Cloud!
Tried that first, it doesn't work. The code seems to really want a deployment to be defined. So even if the environment variables can override, they can't work without a base deployment. So that means having to build the configuration file, and so that might as well have the data in it.
|
On Sep 19 2021, at 3:22 pm, Lukas Malkmus ***@***.***> wrote:
@lukasmalkmus commented on this pull request.
In README.md ***@***.***/0?redirect=https%3A%2F%2Fgithub.com%2Faxiomhq%2Fcli%2Fpull%2F92%23discussion_r711797882&recipient=cmVwbHkrQUFSUVVIT08zV0laNlAyUlRLUUVNRk43S042UFhFVkJOSEhEWEZNQkVNQHJlcGx5LmdpdGh1Yi5jb20%3D):
> @@ -87,9 +87,26 @@ In all cases the installation can be validated by running `axiom -v` in the
terminal:
```shell
+$ axiom -v
The description above states In all cases the installation can be validated by running axiom -v in the terminal. I would prefer to not add this line but get rid of the shell keyword in the code fence.
I guess what I found confusing there is that you're using the shell section for commands elsewhere, and then using this one for output. It seems like if you want to show it is output to the command a shell escape isn't quite right.
|
Feel free to resolve the remaining discussions and update the branch. I enabled auto merging and made the check pass. |
Head branch was pushed to by a user without write access
I switch the build over to using the make file to get the version information. Now it doesn't build on armhf, s390 or ppc. It looks like an issue with the linker options. I've attached a build log below:
|
I'll take another look at this soon. Build failures are indeed related to the flags you pointed out. You can take a look at the differences in build flags for each os/arch in the |
Builds a snap and has a service for shipping the system logs off to Axiom.