-
Notifications
You must be signed in to change notification settings - Fork 86
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
raise report level for some shed linting #1111
base: master
Are you sure you want to change the base?
raise report level for some shed linting #1111
Conversation
I think these are all cases which lead to errors in deploying. fixes https://github.com/galaxyproject/tools-iuc/issues/3384
to make it pass shed_lint without --tool
2f8d4f6
to
bc43d59
Compare
lint_ctx.warn(msg) | ||
lint_ctx.error(msg) | ||
else: | ||
lint_ctx.error("Repository does not define: %s" % key) |
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 think this is going against the original intent of the method _lint_if_present
? E.g. type
is unrestricted
by default, so probably no need to give an error if it's not present.
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.
Maybe we can rename the function? My aim it to error in cases where upload to the toolshed will error ... and I guess this is the case here.shed_lint
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.
OK. I see .. the type does not need to be defined in .shed.yml
.
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.
Is this also the case for name
, owner
, and categories
?
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.
name
and owner
can be specified on the command-line with e.g. shed_update
. name
defaults to the directory name if it's not specified in other ways. Categories I think are optional? I suppose these can all be warnings if not present.
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 think name
and owner
should also be in the realized repo if given on the command line:
planemo/planemo/shed/__init__.py
Line 1022 in 157cace
config = self._realize_config(name) |
name
..)
63e0528
to
4ce7cc4
Compare
4ce7cc4
to
cee8cfb
Compare
@@ -290,14 +290,14 @@ def lint_repository_dependencies(realized_repository, lint_ctx): | |||
def lint_shed_yaml(realized_repository, lint_ctx): | |||
path = realized_repository.real_path | |||
shed_yaml = os.path.join(path, ".shed.yml") | |||
if not os.path.exists(shed_yaml): | |||
lint_ctx.info("No .shed.yml file found, skipping.") | |||
if not os.path.exists(shed_yaml) and realized_repository.repository_type != REPO_TYPE_UNRESTRICTED: |
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.
Not sure I understand this change?
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.
Was an attempt to fix the testing error for https://github.com/galaxyproject/planemo/tree/master/tests/data/repos/suite_1 which fails because there is no .shed.yml
.. which is OK in this case .. or isn't it?
I think these are all cases which lead to errors in deploying.
fixes #1112