Conversation
|
nosborn/github-action-markdown-cli@508d6ce and tcort/github-action-markdown-link-check@a800ad5 are not allowed to be used in openhab/openhab-addons. Actions in this workflow must be: within a repository owned by openhab, created by GitHub, or matching the following: Ana06/get-changed-files@, ghys/checkstyle-github-action@, stCarolas/setup-maven@*. Who can allow this? |
|
Add-ons maintainers should be able to configure GH repo settings. There you can add exceptions. |
|
Yes, it is. :-) |
|
What am I missing? We fixed the 1800+ markdown issues. Re running the tools shows no progress. Looks like it gets old source code?! |
|
There's one more to merge #19028 |
|
Updated the PR, now it is down to 250. Yes one PR to go. I also wonder what happend to freebox, as that binding shows errors while being removed. |
|
Only freeboxos is left. |
|
Does it need to be rebased? |
Freebox and freeboxos are separate bindings. The first is removed and now no longer (after merge to current head) shows no more warnings. I think somehow missed freeboxos. |
Related to openhab#19010 Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
Thanks for all your efforts on this! 👍 I have created #19062 covering FreeboxOS. Now enjoy your holiday. 😎 |
|
Yes you've certainly earned a nice holiday after fixing all these Markdown issues! 😉 |
wborn
left a comment
wborn
left a comment
There was a problem hiding this comment.
Some docs on how to run this locally would also be nice so contributors can more quickly get feedback on their changes and do not have to wait for CI.
Would be nice if it becomes part of the Spotless config but it doesn't seem to support markdownlint.
Another possibility for being able to easily run it locally would be by using the frontend-maven-plugin to run markdownlint.
Maybe it can become part of the CI workflow so we have only have one badge to check for the overal status?
|
There is room for improvement. I have not found a way to run it locally without additional setup steps. Edit: when the frontend-maven-plugin is added to the root pom, will the check be run by all mvn builds even when someone only has the binding subfolder as project opened? If so this would be very useful. Nevertheless this ci action is still needed as gatekeeper when I’m back I will add some lines to the docs (in a separate PR) |
* Fix Markdown Related to #19010 Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
|
All green, ready to merge |
* Fix Markdown Related to openhab#19010 Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
|
Ping @openhab/add-ons-maintainers i prefer not to wait as new markdown issues will then probably be merged. |
|
@wborn - can you have a look? Unfortunately my knowledge here is limited, so I would only be able to blindly approve and merge it and see what happens. |
|
If you address the review comments, I will help fixing any new issuess and test it locally based on the docs. |
* Fix Markdown Related to openhab#19010 Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk> Signed-off-by: Paul Smedley <paul@smedley.id.au>
|
Made some progress. I added markdownlint to the mvn verify step. It is very challenging as the build proces can be started from two places: the project root or a bundle. I managed to fix these working directory issues except for the last step: loading the markdown config file. Any ideas? |
|
Maybe you can use Line 490 in ce138b5 |
|
Thanks, that was exactly where i was looking for. I have now changed the paths and it is more clean now. Errors are interactive/clickable and point to the problematic file straight away. Nice! Tried to run it from a few different bundles, all works as expected. I have removed the workflow as the markdown check is now part of the verify fase, so it should also be run from CI. I have a full build running now to see if that gives any findings. If not i think this is ready to get merged and/or get aditional feedback. Edit: locally the full build is succesfull it detects some markup issues that i will fix in another PR. Edit: Is there some conflict with jsscripting? As the CI fails due to this error: |
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
* Fix wrong command order * Fix jsscripting conflicts Signed-off-by: Wouter Born <github@maindrain.net>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
lsiepel
force-pushed
the
worlfown-markdownlinter
branch
from
a5326b6 to
a2496b3
Compare
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
Signed-off-by: Leo Siepel <leosiepel@gmail.com>
|
@digitaldan could you be of any help here? What am i missing regarding the npm issues. |
|
Hi, I'm traveling for the next week, but will try and take a look when I have a little down time . |
|
@lsiepel on first glance, the |
I have a hard time to figure out what exactly fails and why, the logs are not that clear. The installdirectory seems to be as expected. Atleast when i build the matter binding without these changes, it builds fine and it ends up with the node folder in the matter-server folder, just like this PR. The .gitignore file also shows that this folder is used (and excluded for git). Hopefully you can have a closer look when you are back to your omcputer, i have so many time on this and find troubleshooting this node stuff not that easy. |
6 participants
Changes:
acton Windows to (closely) mimic GHA locallyPlease provide feedback.
Fixes: #13858
Refs: #19011