Closed kingdonb closed 2 years ago
Outside of this one file that differs, I noticed some other differences in Dockerfile
at least. Those look like they should be changes against flux2-multi-tenancy so I will file that as a separate PR.
There are some differences in .github/workflows/test.yaml
as well, not sure how to resolve those yet. I will read the history of each file but in summary:
multi-tenancy repo .github/workflow/test.yaml
:
on:
pull_request:
push:
branches:
- 'main'
this repo:
on:
workflow_dispatch:
pull_request:
push:
branches: [ '*' ]
tags-ignore: [ '*' ]
I don't know if these differences are intentional or significant. I'm not even sure of the significance of workflow_dispatch
in GitHub Actions. Maybe they don't have to be resolved at all. They can be different examples, and there might be different use cases worth demonstrating both. I think it makes sense to run commits from all branches, and pull requests, someone might prefer to ignore or not ignore tags. Someone might think it's better to run only main
branch and any PRs.
I think this PR could be merged as-is, I don't think anything needs to be done about the remaining differences above, except for the Dockerfile PR to multi-tenancy that I will open now, and link back to this one.
workflow_dispatch
allows repo owners to run the workflow from GH UI. I would add everywhere.
Update validate.sh to match https://github.com/fluxcd/flux2-multi-tenancy/commit/0ca0c1ddeb171ddfad55678951e59e8f73dd99f9
I think these files should match. I compared them carefully and found that
maxdepth 2
was changed frommaxdepth 1
in that commitThe position of the argument was updated to conform with what
find
on BSD/Mac machines expects. I don't think it makes a difference in other contexts. However themaxdepth
value was also updated from1
to2
without much discussion. I think it makes as much sense to have the value set2
here as it has been there. 👍I updated this file here so that it matches the newer version from https://github.com/fluxcd/flux2-multi-tenancy/pull/35