Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci-checks: run all the checks on checked-in files; check shellcheck #175

Merged
merged 7 commits into from
Jun 17, 2020

Conversation

grahamc
Copy link
Contributor

@grahamc grahamc commented Jun 17, 2020

Don't run on files which are not being committed, and run all
the checks even if one step failed.

This PR should fail (validating that the CI check still fails) and then I will add additional commits fixing its complaints.

@grahamc
Copy link
Contributor Author

grahamc commented Jun 17, 2020

@mmlb do you think it is worth adding git to the shell.nix for this, or should we go back to using wildcards? I wanted to use git ls-files so that untracked files in a checkout don't gum up the works.

@mmlb
Copy link
Contributor

mmlb commented Jun 17, 2020

I'm fine with adding git to shell.nix. The shell.nix's aren't super pure for the projects and I've kind of wanted to move towards that way as much as possible where it makes sense.

@grahamc grahamc marked this pull request as ready for review June 17, 2020 14:32
@grahamc grahamc requested a review from mmlb June 17, 2020 14:32
if [ $? -eq 0 ]; then
break
fi
while ! docker login -u username -p password localhost; do
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this really belong in this PR? I'm fine if you want to earmark it in, just wand to double check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe not, it just seemed like the easiest fix for $i being unused. I could swap it out to just echo the iteration. What do you think?

@grahamc grahamc added the ready-to-merge Signal to Mergify to merge the PR. label Jun 17, 2020
@mergify mergify bot merged commit c32858a into tinkerbell:master Jun 17, 2020
@grahamc grahamc deleted the shellcheck branch June 17, 2020 19:42
@mmlb mmlb removed the ready-to-merge Signal to Mergify to merge the PR. label Jan 25, 2021
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.

None yet

2 participants