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

Change info message for markdown readme in repo #1398

Merged
merged 3 commits into from
Oct 29, 2023

Conversation

bernt-matthias
Copy link
Contributor

not sure if we should check for correct markdown.

@nsoranzo
Copy link
Member

nsoranzo commented Oct 24, 2023

@mvdbeek To answer your question from Matrix, a README.rst is rendered on the TS 1.0 (see e.g. https://toolshed.g2.bx.psu.edu/repository?repository_id=552adf1ac7cd330f&changeset_revision=18c0086450bb ), while a README.md is not (see e.g. https://toolshed.g2.bx.psu.edu/repository?repository_id=9be0eab5a3639fee&changeset_revision=74810db257cc ).
I cannot check on the TTS (i.e. TS 2.0) since any repository details page loads forever for me today.

So maybe we should keep this check for now?

@mvdbeek
Copy link
Member

mvdbeek commented Oct 24, 2023

What I mean is, this isn't displayed at all in the install process in Galaxy, so you wouldn't ordinarily find it.

@mvdbeek
Copy link
Member

mvdbeek commented Oct 24, 2023

(and having a markdown readme is anyway a bit irrelevant as a check, you should be free to have one whether or not you also have a RST file)

@nsoranzo
Copy link
Member

I think a tool developer may find the "No README found skipping." surprising if they have included a README.md and don't know about TS not supporting it. I'll propose a different wording at info level.

@nsoranzo
Copy link
Member

What about:

diff --git a/planemo/shed_lint.py b/planemo/shed_lint.py
index 38f4ffd4..322d1675 100644
--- a/planemo/shed_lint.py
+++ b/planemo/shed_lint.py
@@ -159,16 +159,15 @@ def lint_readme(realized_repository, lint_ctx):
         if os.path.exists(readme):
             readme_found = readme
 
-    readme_md = os.path.join(path, "README.md")
-    if not readme_found and os.path.exists(readme_md):
-        lint_ctx.warn("Tool Shed doesn't render markdown, " "README.md is invalid readme.")
-        return
-
     if not readme_found:
         # TODO: filter on TYPE and make this a warning if
         # unrestricted repository - need to update iuc standards
         # first though.
-        lint_ctx.info("No README found skipping.")
+        readme_md = os.path.join(path, "README.md")
+        if os.path.exists(readme_md):
+            lint_ctx.info("Found README in Markdown format, which is not rendered by the Tool Shed, skipping")
+        else:
+            lint_ctx.info("No README found, skipping.")
         return
 
     if readme_found.endswith(".rst"):

Co-authored-by: Nicola Soranzo <[email protected]>
@mvdbeek mvdbeek changed the title drop markdown check for markdown Change info message for readme in repo Oct 29, 2023
@mvdbeek mvdbeek merged commit cada8bd into galaxyproject:master Oct 29, 2023
8 of 14 checks passed
@mvdbeek mvdbeek changed the title Change info message for readme in repo Change info message for markdown readme in repo Oct 29, 2023
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.

3 participants