Skip to content
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.

Sign up for GitHub

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

Jump to bottom

updated workflow/test-pr.yaml #30

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

updated workflow/test-pr.yaml #30

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/test-pr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
outputs:
templates: ${{ steps.filter.outputs.changes }}
steps:
- uses: dorny/paths-filter@v2
- uses: dorny/paths-filter@v3
id: filter
with:
filters: |
Expand All @@ -19,6 +19,7 @@ jobs:
needs: [detect-changes]
runs-on: ubuntu-latest
continue-on-error: true
if: ${{ needs.detect-changes.outputs.templates != '[]' && needs.detect-changes.outputs.templates != ''}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is currently working as expected by this PR and the tests are only run on the Templates that were updated. Hence, this PR (which does not update Templates) does not run any tests.

Also, see another example #31 only updated color Template, hence, only tests against color were executed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. this PR is not changing any template, it not supposed to. mostly did this pull request, because I did run into that issue, posted above and I found out how to fix it. I am trying to help my company with devcontainers and I know we/they could run into that error. So this statement would help whomever is learning this process like I to not have to waste energy on non-issues.

but all good, we can close it out if it's not needed. Thank you for your time to review. love the work you do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cog-Smith Is it possible to point me to your workflow run that failed? I am curious to learn why it's failing for your particular scenario, but always passed for other repos/PRs.

Happy to merge this PR if we think it's needed, thanks!

Copy link

@brpaz brpaz Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samruddhikhandale I faced exactly the same problem today, and I fixed in my repo in a similar way, before finding this PR.

I just cloned the template repo and added a new template to the src folder. https://github.com/brpaz/devcontainer-templates/actions/runs/10771852695

I also had to add actions/checkout to clone the repo.

I am not sure how this can be working for other repos. Unless matrix strategy already supported, ignoring a job when receiving an empty list. Maybe it was some change on GitHhub side? 🤷

strategy:
matrix:
templates: ${{ fromJSON(needs.detect-changes.outputs.templates) }}
Expand Down