Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

feature: add the yamlint tool to check check all yaml files #1125

Merged
merged 1 commit into from
Dec 7, 2019

Conversation

fengzixu
Copy link
Collaborator

@fengzixu fengzixu commented Dec 6, 2019

Sign-off-by: fengzixu [email protected]

Ⅰ. Describe what this PR did

I add the yamllint tool to check yaml files in Dragonfly, so that we can ensure that their format is legal.

Ⅱ. Does this pull request fix one issue?

fix dragonflyoss/dragonfly#1112

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@yeya24
Copy link
Collaborator

yeya24 commented Dec 6, 2019

CI failed because of a bad link. I will file a PR for it.

@allencloud
Copy link
Contributor

CI fails:

#!/bin/bash -eo pipefail
find  ./* -name  "*" | xargs misspell -error
xargs: misspell: No such file or directory

Exited with code exit status 127

I have tested the image pouchcontainer/pouchlinter:v0.2.6locally, there is no misspell in it:

~ # docker run -it pouchcontainer/pouchlinter:v0.2.6 bash
root@8c37e24d3126:/#
root@8c37e24d3126:/# which misspell
root@8c37e24d3126:/#

@fengzixu
Copy link
Collaborator Author

fengzixu commented Dec 7, 2019

CI fails:

#!/bin/bash -eo pipefail
find  ./* -name  "*" | xargs misspell -error
xargs: misspell: No such file or directory

Exited with code exit status 127

I have tested the image pouchcontainer/pouchlinter:v0.2.6locally, there is no misspell in it:

~ # docker run -it pouchcontainer/pouchlinter:v0.2.6 bash
root@8c37e24d3126:/#
root@8c37e24d3126:/# which misspell
root@8c37e24d3126:/#

I know. But when I build the image of pouchlinter's master branch on my local env, misspell works fine.

image
Is there any mistake during build pouchlinter:v0.2.6?

@fengzixu
Copy link
Collaborator Author

fengzixu commented Dec 7, 2019

I run the build command in pouchlinter directory: docker build . -t pouchlint

.circleci/config.yml Outdated Show resolved Hide resolved
@fengzixu fengzixu force-pushed the master branch 3 times, most recently from 23ce0c5 to 896897f Compare December 7, 2019 08:20
@pouchrobot pouchrobot added size/M and removed size/XS labels Dec 7, 2019
@fengzixu
Copy link
Collaborator Author

fengzixu commented Dec 7, 2019

image
Some illegal characters cause errors when yamllint tool performed
https://circleci.com/gh/dragonflyoss/Dragonfly/12964

@fengzixu
Copy link
Collaborator Author

fengzixu commented Dec 7, 2019

It looks like the yamllint works fine. But we need to check if some part of rules we need to ignore them.

@yeya24
Copy link
Collaborator

yeya24 commented Dec 7, 2019

@fengzixu Could you please fix all the warning and error?

@fengzixu
Copy link
Collaborator Author

fengzixu commented Dec 7, 2019

@fengzixu Could you please fix all the warning and error?

Doing

@fengzixu fengzixu force-pushed the master branch 2 times, most recently from 0baa819 to f4e9f05 Compare December 7, 2019 12:53
@codecov-io
Copy link

codecov-io commented Dec 7, 2019

Codecov Report

Merging #1125 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1125   +/-   ##
======================================
  Coverage    47.2%   47.2%           
======================================
  Files         116     116           
  Lines        7204    7204           
======================================
  Hits         3401    3401           
  Misses       3535    3535           
  Partials      268     268

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20dca47...a32f2fd. Read the comment docs.

1. add the yamllint into the circleci config
2. format the yaml files according to yamllint

Signed-off-by: fengzixu <[email protected]>
@fengzixu
Copy link
Collaborator Author

fengzixu commented Dec 7, 2019

@yeya24 @allencloud @starnop
I have formated all yaml files according to the suggestion of yamllint. (warning and error)
BYW, I just ignore line-length, because we cannot confirm the length 80 is suitable for our project.

https://circleci.com/gh/dragonflyoss/Dragonfly/12987?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Copy link
Collaborator

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM Thanks

@allencloud
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Dec 7, 2019
@allencloud allencloud merged commit 6235fe1 into dragonflyoss:master Dec 7, 2019
inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019
feature: add the yamlint tool to check check all yaml files
sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature LGTM one maintainer or community participant agrees to merge the pull reuqest. size/XXL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add yamlint in CI system for all the yaml files in this repo
5 participants