Skip to content

Conversation

@jessebot
Copy link
Collaborator

Pull Request

Description of the change

Only run the chart linting action for:

  • charts/nextcloud/Chart.yaml
  • charts/nextcloud/values.yaml
  • charts/nextcloud/templates/** (everything in the templates directory)

Benefits

This is better than trying to manually exclude all the possible things we don't want to run it on.

Possible drawbacks

Can't think of any.

Additional information

This would fix the issue where this PR, #574, is trying to run the chart linter 🤦

Checklist

…every exception

Signed-off-by: jessebot <jessebot@linux.com>
@jessebot jessebot self-assigned this May 29, 2024
@provokateurin
Copy link
Member

Hm now the PR doesn't pass, because the workflow is set to be required. Could you adapt the workflow so that there is a summary job that always like in https://github.com/nextcloud/.github/blob/master/workflow-templates/node.yml? Then we can mark that one as required and we won't accidentally merge with red CI.

@jessebot
Copy link
Collaborator Author

Hm now the PR doesn't pass, because the workflow is set to be required. Could you adapt the workflow so that there is a summary job that always like in https://github.com/nextcloud/.github/blob/master/workflow-templates/node.yml? Then we can mark that one as required and we won't accidentally merge with red CI.

lemme give it a try!

@jessebot
Copy link
Collaborator Author

oh, that did something! :O You're a genius, Kate!

Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

Hm this looks wrong, as the chart testing step should be skipped since the changes in this PR didn't touch any of those files.

Signed-off-by: jessebot <jessebot@linux.com>
@jessebot
Copy link
Collaborator Author

Hm this looks wrong, as the chart testing step should be skipped since the changes in this PR didn't touch any of those files.

You're right, I missed the needs parameter for the lint-test job. The final summary job is currently called Lint and Test Charts, which is an attempt to match the workflow you linked where the workflow itself is called node and the final summary job is also node. Do you want to keep that same formatting? Open to thoughts and opinions :)

@jessebot jessebot requested a review from provokateurin May 29, 2024 13:16
@provokateurin
Copy link
Member

You can just change the name if you like. I'll change which workflow is required afterwards (but it will require every PR to be rebased).

@provokateurin
Copy link
Member

Looks like it is also fine to have the required job being skipped? Then we don't even need the summary job which was probably was just added for compatibility.

@jessebot
Copy link
Collaborator Author

You can just change the name if you like. I'll change which workflow is required afterwards (but it will require every PR to be rebased).

oof, rebasing every PR in this repo will be pain.

Looks like it is also fine to have the required job being skipped? Then we don't even need the summary job which was probably was just added for compatibility.

I'm confused. How do I do that? 🤔

@provokateurin
Copy link
Member

oof, rebasing every PR in this repo will be pain.

I think we actually don't need that, as the name of the required job won't change.

Signed-off-by: jessebot <jessebot@linux.com>
@jessebot jessebot requested a review from provokateurin May 29, 2024 13:43
@jessebot jessebot merged commit ba6ce9d into nextcloud:main May 29, 2024
@jessebot jessebot deleted the update-chart-linting branch May 29, 2024 13:58
SwitzerChees pushed a commit to SwitzerChees/helm that referenced this pull request Jun 27, 2024
…every exception (nextcloud#575)

* only run chart linting for specific files instead of trying to catch every exception

Signed-off-by: jessebot <jessebot@linux.com>

* attempt to adapt changes from https://github.com/nextcloud/.github/blob/master/workflow-templates/node.yml

Signed-off-by: jessebot <jessebot@linux.com>

* add needs: changes to lint job

Signed-off-by: jessebot <jessebot@linux.com>

* remove summary job afterall

Signed-off-by: jessebot <jessebot@linux.com>

---------

Signed-off-by: jessebot <jessebot@linux.com>
Signed-off-by: switzerchees <patrickmichel96@yahoo.de>
raynay-r pushed a commit to raynay-r/nextcloud-helm that referenced this pull request Jun 28, 2024
…every exception (nextcloud#575)

* only run chart linting for specific files instead of trying to catch every exception

Signed-off-by: jessebot <jessebot@linux.com>

* attempt to adapt changes from https://github.com/nextcloud/.github/blob/master/workflow-templates/node.yml

Signed-off-by: jessebot <jessebot@linux.com>

* add needs: changes to lint job

Signed-off-by: jessebot <jessebot@linux.com>

* remove summary job afterall

Signed-off-by: jessebot <jessebot@linux.com>

---------

Signed-off-by: jessebot <jessebot@linux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants