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

Add yaml linter #3979

Merged
merged 5 commits into from
Jun 28, 2022
Merged

Add yaml linter #3979

merged 5 commits into from
Jun 28, 2022

Conversation

asantos00
Copy link
Contributor

Description of changes:
This adds the capacity to lint yaml files. I would love to hear your opinion on this, if it makes sense, if it looks correct, and so on.

It also adds a browserified version of yaml-lint (https://www.npmjs.com/package/yaml-lint) that is needed in yaml-lint.js

Notes

  • Still missing tests (will add them later today)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Jun 3, 2019

Codecov Report

Merging #3979 (56b591b) into master (fbe3b69) will increase coverage by 0.44%.
The diff coverage is 86.56%.

@@            Coverage Diff             @@
##           master    #3979      +/-   ##
==========================================
+ Coverage   69.50%   69.94%   +0.44%     
==========================================
  Files         517      559      +42     
  Lines       50163    58336    +8173     
  Branches     9461    11233    +1772     
==========================================
+ Hits        34866    40805    +5939     
- Misses      15297    17531    +2234     
Flag Coverage Δ
unittests 69.94% <86.56%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/ace/mode/yaml/yaml-lint.js 40.91% <ø> (ø)
lib/ace/mode/yaml.js 75.00% <77.77%> (+4.62%) ⬆️
lib/ace/mode/yaml_worker.js 84.00% <84.00%> (ø)
lib/ace/mode/yaml_worker_test.js 90.90% <90.90%> (ø)
lib/ace/mode/vbscript.js 88.04% <0.00%> (-11.96%) ⬇️
lib/ace/worker/mirror.js 51.35% <0.00%> (-9.26%) ⬇️
lib/ace/mode/ruby.js 89.09% <0.00%> (-4.25%) ⬇️
lib/ace/lib/app_config.js 73.86% <0.00%> (-4.19%) ⬇️
lib/ace/layer/cursor.js 84.13% <0.00%> (-2.20%) ⬇️
lib/ace/keyboard/sublime.js 55.68% <0.00%> (-1.47%) ⬇️
... and 213 more

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 fbe3b69...56b591b. Read the comment docs.

@nightwing
Copy link
Member

nightwing commented Jun 4, 2019

Thanks, this looks good! Could you tell what command did you use to create the browserified version?

@asantos00
Copy link
Contributor Author

Hi @nightwing, thank you! I think I've used browserify yaml-lint.js --outfile yaml-lint.js --standalone lint. What am I supposed to use? Because I see I'm having some problems on the build 😞

@rushtehrani
Copy link

Is there a plan to merge this?

@lynette-li
Copy link

@nightwing
Is there a plan to merge this?
I need this feature

@asantos00
Copy link
Contributor Author

I guess I need to do some changes but I'd need help to understand what should I do in order to get a proper browserified version 🤷‍♂

@huizluo
Copy link

huizluo commented Dec 15, 2020

i need this feature

@andrewnester andrewnester reopened this Jun 28, 2022
@andrewnester andrewnester self-requested a review June 28, 2022 13:53
@andrewnester
Copy link
Contributor

@asantos00 I think everything is okay with your browserfied version, you can just add it to this file so eslint won't validate it
https://github.com/ajaxorg/ace/blob/master/.eslintignore

@asantos00
Copy link
Contributor Author

@andrewnester sorry, my bad I didn't notice. I've added that to eslintignore, let me know if you need any other changes (also I need your approval for the workflows to run)

@andrewnester andrewnester merged commit 451f915 into ajaxorg:master Jun 28, 2022
@andrewnester
Copy link
Contributor

Merged! Thanks a lot for your contribution

@asantos00 asantos00 deleted the add-yaml-linter branch June 28, 2022 15:10
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.

6 participants