-
Notifications
You must be signed in to change notification settings - Fork 110
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
Save build log #237
base: master
Are you sure you want to change the base?
Save build log #237
Conversation
It was pseudo code ;P
if you only want to save |
.github/workflows/linux.yaml
Outdated
if: always() | ||
with: | ||
name: log-file | ||
path: *.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path: *.log | |
path: test-suite.log |
.github/workflows/linux.yaml
Outdated
if: always() | ||
with: | ||
name: log-file | ||
path: *.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path: *.log | |
path: test-suite.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Etc. etc. for the rest as well
ee5c3e7
to
d554454
Compare
I also suggest to add different names for the artifacts, otherwise they will collide. |
.github/workflows/linux.yaml
Outdated
- uses: actions/upload-artifact@v4 | ||
if: always() | ||
with: | ||
name: log-file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use a unique name like alpine-${{ matrix.compiler }}-log-file
.
.github/workflows/linux.yaml
Outdated
- uses: actions/upload-artifact@v4 | ||
if: always() | ||
with: | ||
name: log-file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use a unique name like debian-${{ matrix.compiler }}-log-file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And so on for all the distros !
On Thu, Dec 05, 2024 at 01:43:54PM -0800, Adrien RICCIARDI wrote:
> @@ -31,6 +36,11 @@ jobs:
CC: ${{ matrix.compiler }}
run: |
test/test_build.sh debian
+ - uses: ***@***.***
+ if: always()
+ with:
+ name: log-file
Maybe use a unique name like debian-${{ matrix.compiler }}-log-file.
Work in progress ...
|
d554454
to
b56d829
Compare
.github/workflows/linux.yaml
Outdated
- uses: actions/upload-artifact@v4 | ||
if: always() | ||
with: | ||
name: ubuntu-${{ matrix.compiler }}-log-file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I forgot that there were 2 Ubuntu versions !
I suggest ubuntu-${{ matrix.ubuntu_version }}-${{ matrix.compiler }}-log-file
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can even drop the initial ubuntu
, using only ${{ matrix.ubuntu_version }}-${{ matrix.compiler }}-log-file
.
For the record, with input from Melroy van den Berg and RICCIARDI-Adrien.
b56d829
to
b567057
Compare
On Thu, Dec 05, 2024 at 02:20:48PM -0800, Adrien RICCIARDI wrote:
You can even drop the initial `ubuntu`,
using only `${{ matrix.ubuntu_version }}-${{ matrix.compiler }}-log-file`.
Nice. Done!
Groeten
Geert Stappers
--
Silence is hard to parse
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Artifacts are present in the workflow view, this looks good to me !
@RICCIARDI-Adrien: What do you think? |
I think that Neustradamus should try their/her/his privilege to approve merge requests for this project. |
Sorry that my approval doesn't count 😜 |
@stappersg: I know but I do not use my "power" ^^ |
@Neustradamus We all agree to merge this PR, what are we waiting for ? |
On Fri, Dec 06, 2024 at 01:26:09PM -0800, Neustradamus wrote:
@stappersg: I know but I do not use my "power" ^^
As uncle Ben once said "With great power comes great responsebility."
@Neustradamus: Please take your responsebility.
Groeten
Geert Stappers
--
Silence is hard to parse
|
On Sat, Dec 07, 2024 at 09:07:59PM -0800, Neustradamus wrote:
@Neustradamus approved this pull request.
Thank you.
Please do what I did in #240, click on 'Merge pull request'.
Groeten
Geert Stappers
--
Silence is hard to parse
|
That depends how you want to work together and what your WoW is. I normally like to merge my own PRs myself, to ensure a PR isn't merge too fast, causing possible regression for example. |
At least an attempt to save build log.
Actual need was saving ./test-suite.log