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

Bs badges envs #143

Merged
merged 7 commits into from
May 8, 2019
Merged

Bs badges envs #143

merged 7 commits into from
May 8, 2019

Conversation

sbreiff
Copy link
Contributor

@sbreiff sbreiff commented May 8, 2019

  • add biosample badge checks to data and staging
  • remove tier1_metadata_missing badge check/action

@sbreiff sbreiff requested a review from aschroed May 8, 2019 15:00
@coveralls
Copy link

coveralls commented May 8, 2019

Coverage Status

Coverage increased (+0.3%) to 91.566% when pulling 78597c4 on bs_badges_envs into 748287c on master.

Copy link
Member

@aschroed aschroed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look fine. One possible suggestion to think on that you can take or leave and a question that I am not sure of.
At line 197 in badge_checks.py you have '0' as the default passage_number if it's missing. I realize that the missing passage number warning will be generated but in this case where passage_number is missing for a stem cell perhaps a sterner specific warning is in order?

The question is that it looks like the Gold commendation check depends on the warnings check to have been run? Do we have any control on the order of check running - I think maybe that adding the warning check to the dependencies list of the commendation may do that but not sure exactly how that works - but is it not possible that some things may inappropriately get commended depending on the order they are run in?

@sbreiff sbreiff merged commit 3b26bde into master May 8, 2019
@sbreiff sbreiff deleted the bs_badges_envs branch September 30, 2019 20:31
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.

None yet

3 participants