-
-
Notifications
You must be signed in to change notification settings - Fork 337
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
g.extension: fix getting addon directories names without Py/C source code #4900
base: main
Are you sure you want to change the base?
g.extension: fix getting addon directories names without Py/C source code #4900
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. You may consider adding a test for this case!?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested some changes for some small typos that were obvious to me. I didn't really analyze if the messages make sense in the context, I limited myself to the words
Test is included (I extended it with commit bc19b1f): grass/scripts/g.extension/testsuite/test_addons_download.py Lines 218 to 234 in 46d2bac
|
At one point we'll need to think about what to do when html man pages might not be the only thing. When using markdown as source, will the addon install a "compiled" page that will HTML (processed from the markdown), or only the source file? |
Looks like the test failed:
|
I don't know what's wrong in the CI. Locally it doesn't show problems:
|
On the local emulator terminal (during tests especially g.extension module) is printed to the stderr GRASS GIS command g.extension module output and debug info too. I enable debug mode beacuse I'm checking some debug variable value Under CI is printed only debug info (not g.extension module output) and therefore this 'test_github_download_official_module_src_code_only' test fails. |
Do we need to modify the CI test? |
Fixes #4761.