Conversation
mdellweg
force-pushed
the
cleanup
branch
2 times, most recently
from
5326a81 to
73f4ef9
Compare
pedro-psb
left a comment
pedro-psb
left a comment
There was a problem hiding this comment.
There are lot of changes in the before_install / install scripts. From reading it that looks fine, but I would like to see a test PR.
Btw, looking much better, thanks!
| echo "No formatting change needed" | ||
| fi | ||
| fi | ||
|
|
There was a problem hiding this comment.
The convenience about this (idea) was having a separate commit for formatting, because the CI update PRs squash all changes from plugin-templates in a single PR. But In the end I think this wasn't even working properly, since we have a format call on the plugin-template apply itself.
| if: "steps.create_pr_changelog.outputs.pull-request-number" | ||
| env: | ||
| GH_TOKEN: "{{ '${{ secrets.RELEASE_TOKEN }}' }}" | ||
| continue-on-error: true |
There was a problem hiding this comment.
We've seen some flaky bugs with changes aggregation but 99% it just works.
And we've reviewed the fragment on the PRs already. I'm fine with that.
|
|
||
|
|
||
| def check_pyproject_dependencies(repo, from_commit, to_commit): | ||
| def check_pyproject_dependencies(repo: Repo, from_commit:str, to_commit:str) -> list[str]: |
There was a problem hiding this comment.
Why black didn't complain about that from_commit:str in the CI here?
If this file is applied at the target repository and we run black against it, it will fail.
| return 1 | ||
|
|
||
| branches = [branch for branch in available_branches if branch in branches] | ||
| branches.reverse() |
There was a problem hiding this comment.
Reverse doesn't sort the branches. Maybe .sort(reverse=True)?
|
|
||
| {% include 'header.j2' %} | ||
|
|
||
| set -euo pipefail |
There was a problem hiding this comment.
Can you put a headline comment here with what is the responsability of this script.
Similar to how you've added in templates/github/.github/workflows/scripts/before_script.sh.j2
| cmd_prefix bash -c "usermod -a -G wheel pulp" | ||
| echo | ||
| echo "# pip list outside the container" | ||
|
|
There was a problem hiding this comment.
I don't see the pip list foor outside the container.
There was a problem hiding this comment.
Hmm, it's on the install.sh. I guess before_script is after install, right? We can move it here, then.
There was a problem hiding this comment.
It would be nice to have the github grouping thing, if you agree.
echo ::group::GROUP_NAME
...
echo ::endgroup::
2 participants
No description provided.