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.

Sign up for GitHub

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

Jump to bottom

g.extension: fix getting addon directories names without Py/C source code #4900

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

g.extension: fix getting addon directories names without Py/C source code #4900

wants to merge 4 commits into from

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented Dec 31, 2024

Fixes #4761.

@tmszi tmszi added bug Something isn't working Python Related code is in Python labels Dec 31, 2024
@tmszi tmszi added this to the 8.4.1 milestone Dec 31, 2024
@tmszi tmszi self-assigned this Dec 31, 2024
@tmszi tmszi mentioned this pull request Dec 31, 2024
Open
ninsbl
ninsbl previously approved these changes Dec 31, 2024
View reviewed changes
Copy link
Member

@ninsbl ninsbl left a 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!?!

@echoix echoix changed the title g.extension: fix getting addon dicrectories names without Py/C src code g.extension: fix getting addon directories names without Py/C source code Dec 31, 2024
echoix
echoix reviewed Dec 31, 2024
View reviewed changes
Copy link
Member

@echoix echoix left a 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

scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved
scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved
scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved
@tmszi
Copy link
Member Author

tmszi commented Jan 3, 2025

Looks good. You may consider adding a test for this case!?!
Test is included

Looks good. You may consider adding a test for this case!?!

Test is included (I extended it with commit bc19b1f):

grass/scripts/g.extension/testsuite/test_addons_download.py

Lines 218 to 234 in 46d2bac

def test_github_install_official_multimodule_and_check_metadata(self):
"""Test installing multi-module extension from official addons
repository without printing warning no metadata available message
for module wich install HTML page file only"""
extension = "i.sentinel"
gextension = SimpleModule(
"g.extension",
extension=extension,
prefix=str(self.install_prefix),
)
self.assertModule(gextension)
self.assertTrue(gextension.outputs.stderr)
self.assertNotIn(
_("No metadata available for module '{}':").format(extension),
gextension.outputs.stderr,
)

@github-actions github-actions bot added the tests Related to Test Suite label Jan 3, 2025
@echoix
Copy link
Member

echoix commented Jan 3, 2025

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@echoix echoix echoix approved these changes

@ninsbl ninsbl ninsbl left review comments

Assignees

@tmszi tmszi

Labels
bug Something isn't working general module Python Related code is in Python tests Related to Test Suite
Projects
None yet
Milestone
8.4.1
Development

Successfully merging this pull request may close these issues.

[Bug] i.wi: does not install
3 participants
@tmszi @echoix @ninsbl