-
Notifications
You must be signed in to change notification settings - Fork 133
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
add linter for noarch #134
Conversation
bioconda_utils/lint_functions.py
Outdated
('python' in deps) and | ||
not _has_preprocessing_selector(meta) | ||
) and ( | ||
'noarch' not in meta['build'] |
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.
Does above line also work if meta has no build section?
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.
thanks, fixed
if ( | ||
('gcc' not in deps) and | ||
('python' in deps) and | ||
not _has_preprocessing_selector(meta) |
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.
What is the point of the preprocessing selector? Is it just because we cannot be sure in that 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.
Right. I was thinking if there was something strange enough in the recipe that it needs preprocessing selectors then it's not a good candidate for noarch.
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.
Sounds reasonable.
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'm worried that this disables the linter for too many packages. I think there are a few packages with pre-processing selectors in the url
section or test section - still we would like to have noarch packages for it. Just a feeling, not sure how often these cases are.
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.
Sorry didn't get a chance to reply before -- If there's a pre-processing selector in the url or test section, doesn't that mean it's arch-specific?
Either way I think it's ok at least for now to let some packages slide until we get a better feel for what works with noarch, and then tighten up the linter later.
Thanks @daler! |
@johanneskoester @bgruening anything else you can think of to look for in the linter?