deployment: fix trailing whitespace errors. #169
Conversation
klihub
commented
Oct 23, 2023
•
klihub
commented
- fix trailing WS errors that slipped through in Helm Chart READMEs
Signed-off-by: Krisztian Litkey <[email protected]>
klihub
force-pushed
the
fixes/trailing-ws-errors
branch
from
a068b0a to
7a1d7b7
Compare
There was a problem hiding this comment.
It looks good to me but I have a question. I'm totally new to githooks so it might sound stupid what I ask. What's the benefit of having git hook way of running checks to catch trailing whitespaces over having a workflow step that would run a script for that?
| - name: Git-check PR for conflict markers and trailing whitespace errors. | ||
| if: ${{ github.event_name == 'pull_request' }} | ||
| run: | | ||
| git fetch -q origin ${{ github.base_ref }} | ||
| if ! git diff-index --check origin/${{ github.base_ref }} --; then | ||
| echo "This PR would introduce the above errors." | ||
| echo "Please fix them and update the PR." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
I was first to comment out if we really need L31 (if operator) assuming we run checks only on pull requests. But in the next steps below, I see that we are using this workflow to build the site. Maybe later on we need to seperate site building steps from verification/tests against pull requests.
There was a problem hiding this comment.
I was first to comment out if we really need L31 (if operator) assuming we run checks only on pull requests. But in the next steps below, I see that we are using this workflow to build the site. Maybe later on we need to seperate site building steps from verification/tests against pull requests.
We can do that, but some consideration should be given whether to go with a large number of small workflows, which again tend to waste common shared resources, or larger blocks which can be customized... and ideally are decomposed into smaller reusable 'sub-workflows' as demonstrated by how @marquiz rewrote the CRI Resource Manager workflows using on/workflow_call/inputs and uses tags together with locally maintained common sub-workflows.
|
Also, it would be good to rename the PR title because it is too long and mix of commit titles I think. |
The idea is to provide a low barrier for checking things that are either wrong (eg. accidentally committed conflict markers), or undesired and probably caught by reviewers, therefore preventing the PR from getting merged. OR to put in another way, to avoid (mis)using scarce common infra resources (shared github workers) to perform problems that can be trivially detected locally. This goes in the same bucket as our |
@fmuyassarov Sorry for the inconvenience but I split out all but the whitespace fixup commit to #171. |
klihub
changed the title
Rewrote the title. |
