Linter pass thorough Addons #2054
Replies: 34 comments
-
Great work Brad! |
Beta Was this translation helpful? Give feedback.
-
Wow. Big thanks. 👍 |
Beta Was this translation helpful? Give feedback.
-
UI addons complete (there was only one...). Now starting on io addons. |
Beta Was this translation helpful? Give feedback.
-
io addons done. voice addons done. linter pass through addons done? @ThomDietrich what next? |
Beta Was this translation helpful? Give feedback.
-
all openhab addons or the openhab 2 addons? |
Beta Was this translation helpful? Give feedback.
-
All OH2 I guess. I will go find the OH1 addons next... |
Beta Was this translation helpful? Give feedback.
-
https://github.com/openhab/openhab-docs/projects/1 I have started a progress card for all locations and i will go through all repos and see what i can find. |
Beta Was this translation helpful? Give feedback.
-
Very nice |
Beta Was this translation helpful? Give feedback.
-
Hey Brad, did you submit any additional pull requests already? |
Beta Was this translation helpful? Give feedback.
-
Hi again, I have just pulled the latest oh2 addons master and my atom editor still showed some linter warnings, when i checked the first 2 bindings. |
Beta Was this translation helpful? Give feedback.
-
And one reminder for me: Code fences are done this way: |
Beta Was this translation helpful? Give feedback.
-
Hi Jerome, Okay on the linter rules. Yes, this is exactly what I did, but perhaps I need to pull in the latest version. I will do that before I work on the next set of PRs. Okay on the code fences. I was a little unclear before, but your note is super-clear. Thank you. |
Beta Was this translation helpful? Give feedback.
-
Hi, We have dicussed the code fences in some PR but i can't find it anymore without hours of searching. About openhab2-addons repo: |
Beta Was this translation helpful? Give feedback.
-
So I wonder about the rules... Take a look at this PR - openhab/openhab1-addons/#5410. In the Satel README.md, I followed your code fence rules above, but the highlighting in Atom for text that is outside of the fences is still affected. Look toward the end of the file. Is this how it should be? |
Beta Was this translation helpful? Give feedback.
-
I see no issue with the changed fences. just some sitemap demo that has not been improved. I am more concerned about the lists. |
Beta Was this translation helpful? Give feedback.
-
Sorry for pushing here again and fpr poking @ThomDietrich again, but this time you have to explain me the behavior. I have no idea of whats going on here: (Have a look at the corresponding line numbers please) First lines are the following: If i add a space, it changes the "add-warning" and also produces a "remove-warning". |
Beta Was this translation helpful? Give feedback.
-
Seems that (remark-lint:list-item-bullet-indent) is causing trouble sometimes. |
Beta Was this translation helpful? Give feedback.
-
What file are you working with? I will take a look at it. |
Beta Was this translation helpful? Give feedback.
-
Development tutorial. I am currently fixing all linter errors in the dev tutorial. |
Beta Was this translation helpful? Give feedback.
-
Hey guys! Regarding the linting rules: They are mostly following the style guide over at http://www.cirosantilli.com/markdown-style-guide Don't hesitate to tune the linting rules! One last thing: @Confectrician you wrote that |
Beta Was this translation helpful? Give feedback.
-
Thanks for the clarificatoin @ThomDietrich and no need for excuses. Topic related: I will try to improve the lists as they are and leave those warning open for now. One last thing: |
Beta Was this translation helpful? Give feedback.
-
FWIW, I have been able to resolve almost all of the linter errors. In the few cases where I could not, I have made a note in the PR, thinking we can go back and fix it later. Also, I am getting ready to dive into the OH1.x binding addons next, but I have a little clean up to do on the 1.x actions I have already contributed. |
Beta Was this translation helpful? Give feedback.
-
@Confectrician, the following fixes the linter errors associated with the lines you referenced...
Basically change the "*" to a "-". After that, the complaints about extra spaces required/not required go away. |
Beta Was this translation helpful? Give feedback.
-
Hey guys, while going through https://github.com/openhab/openhab2-addons/pull/3055 I found another linter exception: https://github.com/openhab/openhab2-addons/pull/3055/files#r159464423 |
Beta Was this translation helpful? Give feedback.
-
Hey Brad, I have updated the project with some infos about the stuff i have already finished. If you need more infos about how to edit the project, feel free to ask me. BR |
Beta Was this translation helpful? Give feedback.
-
Hey Jerome, Brad |
Beta Was this translation helpful? Give feedback.
-
Sigh... I finally got some time to do some testing, and found that I had not installed the linter correctly for the add-ons directory. This is why you are seeing so many problems in my PRs. Jeez - one more step up the learning curve. So @Confectrician you are right to be going back through all of my submissions in the add-ons. Jerome, since you are already working back through the OH2 addons, how about if I work my way back through the OH1 addons, starting with Actions. I will then work through the OH1 Bindings. I feel bad that I did not understand how the linter works, but that is the way it goes. One question for you or for @ThomDietrich, now that I have installed the linter in the OH1 directory and have entered the linter installation stuff in |
Beta Was this translation helpful? Give feedback.
-
Usually the .gitignore in .gitignore should work, but it doesn't work for me too at the moment. |
Beta Was this translation helpful? Give feedback.
-
Nooo 😅 don't do that guys :) Back to the issue: I suspect you want to ignore the package.json file during staging/commit? Simply said this is not possible. You'd either need to remove it or ignore it in gitignore. Both are not what we ultimately want, so the best option is really to manually ignore it. Yes that means not using git status
git add addons/*/*/README.md
git status # now use your brain :D Btw great overall progress from both of you, thanks again! |
Beta Was this translation helpful? Give feedback.
-
Yep - makes sense to me. Since I am only working on one thing at a time, the -a flag makes a lot of sense. If I were doing code development and touching a number of files while working on a project, then it would be different. Hum - using brain is required. I had no idea I would have to use my brain as part of this project ;-). |
Beta Was this translation helpful? Give feedback.
-
I have finished the bindings. I am starting on the UI addons next.
Beta Was this translation helpful? Give feedback.
All reactions