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

feature: yaml lint #1068

Merged
merged 2 commits into from
Oct 18, 2021
Merged

feature: yaml lint #1068

merged 2 commits into from
Oct 18, 2021

Conversation

sheeeng
Copy link
Contributor

@sheeeng sheeeng commented Sep 23, 2021

$ yamllint --config-file=.github/linters/.yaml-lint.yml .
./.codeclimate.yml
  29:17     warning  empty value in block mapping  (empty-values)
  32:17     warning  empty value in block mapping  (empty-values)

./deploy/aws-lambda/serverless.yml
  30:2      warning  wrong indentation: expected 2 but found 1  (indentation)

./deploy/docker-compose/insecure/docker-compose.yml
  9:1       error    trailing spaces  (trailing-spaces)

./deploy/docker-compose/with-nginx-proxy-and-letsencrypt/docker-compose.yml
  45:9      error    trailing spaces  (trailing-spaces)
  46:9      warning  empty value in block mapping  (empty-values)
  47:11     warning  empty value in block mapping  (empty-values)
  48:8      warning  empty value in block mapping  (empty-values)
  49:11     warning  empty value in block mapping  (empty-values)

./.github/workflows/codeql-analysis.yml
  3:1       warning  truthy value should be one of [false, true]  (truthy)
  18:5      warning  wrong indentation: expected 6 but found 4  (indentation)
  48:6      warning  missing starting space in comment  (comments)

./.github/workflows/test.yml
  2:1       warning  truthy value should be one of [false, true]  (truthy)
  42:7      warning  wrong indentation: expected 4 but found 6  (indentation)

./.github/workflows/push.yml
  1:1       warning  truthy value should be one of [false, true]  (truthy)
  11:5      warning  wrong indentation: expected 6 but found 4  (indentation)

@codecov
Copy link

codecov bot commented Oct 6, 2021

Codecov Report

Merging #1068 (bbd069a) into master (b82084b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1068   +/-   ##
=======================================
  Coverage   78.16%   78.16%           
=======================================
  Files           6        6           
  Lines         403      403           
=======================================
  Hits          315      315           
  Misses         58       58           
  Partials       30       30           

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 61e6da0...bbd069a. Read the comment docs.

@sheeeng
Copy link
Contributor Author

sheeeng commented Oct 7, 2021

Fixed once again with the correct upstream master branch.

$ git remote add upstream [email protected]:jhaals/yopass.git
$ git remote --verbose show upstream
* remote upstream
  Fetch URL: [email protected]:jhaals/yopass.git
  Push  URL: [email protected]:jhaals/yopass.git
$ git fetch --all --prune && git pull --rebase --prune upstream master

.github/workflows/test.yml Outdated Show resolved Hide resolved
@@ -47,7 +54,7 @@ jobs:
- uses: cypress-io/github-action@v2
with:
start: yarn start
wait-on: 'http://localhost:3000'
wait-on: "http://localhost:3000"
Copy link
Owner

Choose a reason for hiding this comment

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

are there a specific reason to why double quotes are favoured?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is true that YAML Specification does not indicate whether double quotes or single quotes are preferred. You can decide on which one this project should use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jhaals, please let me know if double quotes or single quotes is the one that you preferred in this project. I will reflect that in later changes. Have a nice weekend.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the quick turnaround @sheeeng!
I'd say let's go with single quotes and then 🚢 this PR :)

Copy link
Contributor Author

@sheeeng sheeeng Oct 15, 2021

Choose a reason for hiding this comment

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

@jhaals, I have quickly checked that both widely used redhat.vscode-yaml (6,629,666 installs) and esbenp.prettier-vscode (15,769,891 installs) VSCode extensions have double quotes for saving YAML files automatically. These two VSCode extensions comes handy while working with YAML files. Thus, the double quotes changes were included in this PR. Sorry for the late information.

Please confirm that you still prefer single quotes and I will modify the lint rules accordingly.

  "[yaml]": {
    "editor.defaultFormatter": "esbenp.prettier-vscode",
    "_commented_editor.defaultFormatter": "redhat.vscode-yaml"
  },

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the additional context, I don't have strong preferences in this case so let's go with this

@sheeeng sheeeng marked this pull request as draft October 15, 2021 08:33
@sheeeng sheeeng marked this pull request as ready for review October 15, 2021 12:54
@sheeeng sheeeng requested a review from jhaals October 15, 2021 12:54
@jhaals jhaals merged commit 3ba9534 into jhaals:master Oct 18, 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.

2 participants