Skip to content

Conversation

@blacha
Copy link
Member

@blacha blacha commented Mar 18, 2025

Motivation

We are somewhat inconsitent in our yaml styles, some things are quoted where they shouldn't need to be quoted.

This is a first draft of applying a linter, I have tried to use the default rules where they don't conflict with prettier as we already use prettier for the yaml formatting.

Modifications

Installs eslint-plugin-yml yaml linter and adds it to eslint to be run on every pull request

Verification

build still builds, testing jobs still runs

@paulfouquet
Copy link
Collaborator

paulfouquet commented Mar 19, 2025

We should probably not trust too much the argo lint --offline templates/ workflows/ and actually test the workflows modified by this PR before merging?

build still builds, testing jobs still runs

maybe we need to run some of the workflowTemplate too?

@blacha
Copy link
Member Author

blacha commented Mar 19, 2025

We should probably not trust too much the argo lint --offline templates/ workflows/ and actually test the workflows modified by this PR before merging?

build still builds, testing jobs still runs

maybe we need to run some of the workflowTemplate too?

I agree, do we have any easy way to run tests off a pull request?

@blacha blacha marked this pull request as ready for review March 19, 2025 23:30
@blacha blacha requested review from a team as code owners March 19, 2025 23:30
@paulfouquet
Copy link
Collaborator

paulfouquet commented Mar 20, 2025

We should probably not trust too much the argo lint --offline templates/ workflows/ and actually test the workflows modified by this PR before merging?

build still builds, testing jobs still runs

maybe we need to run some of the workflowTemplate too?

I agree, do we have any easy way to run tests off a pull request?

That would be nice to do something like what have been removed here but deleting the "test" namespace only when the PR is merged so during the time the PR is opened we can test the workflows on that namespace.

Copy link
Collaborator

@paulfouquet paulfouquet left a comment

Choose a reason for hiding this comment

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

I don't think it's necessary to apply these changes on the examples in the different readmes, but they may be inconsistent in their format now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants