Conversation
Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
…ondition Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
…support Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
…results Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
Signed-off-by: Kosta Ilic <kosta.ilic@emerson.com>
|
@bkeryan, I have some reservations about the nomenclature in various files. I'd like to get feedback on the overall approach before spending time on terminology improvements. |
| needs: [checks_succeeded] | ||
| report_test_results: | ||
| name: Report test results | ||
| name: Test Results |
There was a problem hiding this comment.
The ruleset lists Test Results as a required check in order to ensure that the tests pass. You can't see it because are not a repo admin, but it's documented here: https://dev.azure.com/ni/DevCentral/_wiki/wikis/AppCentral.wiki/139879/GitHub-Branch-Protection-Rules-and-Rulesets
You created two other checks with this name, but I don't think either one guarantees that the tests pass.
| event_file: | ||
| name: "Event File" | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Upload | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: Event File | ||
| path: ${{ github.event_path }} |
There was a problem hiding this comment.
The point of doing this is so that you can run EnricoMi/publish-unit-test-result-action from a separate job that is invoked by
on:
workflow_run:
workflows: ["CI"]
types:
- completed
I don't see that anywhere in this PR.
| run: ls -lR | ||
| - name: Publish test results | ||
| uses: EnricoMi/publish-unit-test-result-action@34d7c956a59aed1bfebf31df77b8de55db9bbaaf # v2.21.0 | ||
| uses: EnricoMi/publish-unit-test-result-action@v2 |
There was a problem hiding this comment.
This is less secure and there is no reason to change it.
| event_file: | ||
| name: "Event File" | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Upload | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: Event File |
There was a problem hiding this comment.
BTW, everything you added uses different naming style than the rest of the workflows
job & step names: Sentence case
job & step ids: snake_case
action/workflow parameters: kebab-case
artifacts: snake_case
|
The problem I attempted to solve was not critical - I was able to make contributions in spite of the inconvenience. I don't want to invest the time to learn enough to properly address the problem at this time, so I am closing the PR. |
2 participants
What does this Pull Request accomplish?
Fix for bug #243. I followed the instructions from the bug report itself, but those did not fully resolve the issue, so I made few additional changes.
Why should this Pull Request be merged?
Refer to the section above.
What testing has been done?
Everything passed in a draft PR out of my fork.
I don't have the access to test this in main. I can use some help there.