-
Notifications
You must be signed in to change notification settings - Fork 714
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
{bio}[foss/2020b} pangolin v3.1.10 #13733
{bio}[foss/2020b} pangolin v3.1.10 #13733
Conversation
}), | ||
('constellations', '0.0.13', { | ||
'source_urls': ['https://github.com/%(github_account)s/%name)s/archive'], | ||
'sources': [{'download_filename': 'v%(version).tar.gz', 'filename': '%(name)s-%(version)s.tar.gz'}], |
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.
'sources': [{'download_filename': 'v%(version).tar.gz', 'filename': '%(name)s-%(version)s.tar.gz'}], | |
'sources': [{'download_filename': 'v%(version)s.tar.gz', 'filename': '%(name)s-%(version)s.tar.gz'}], |
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 for finding them! I simply did not spot them. (I do need new glasses, not a joke). I have corrected them.
By the way, is there a way to avoid these repeating lines?
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.
Converted to draft as we are waiting to have UShER
merged.
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.
By the way, is there a way to avoid these repeating lines?
You can likely put them into exts_default_options
(dict that is merged with this dict) although I'm not fully sure it works with the templates, but please try.
Also use SOURCE_TAR_GZ
as the filename
and GITHUB_SOURCE
as the source_url, see https://docs.easybuild.io/en/latest/version-specific/easyconfig_templates.html
Converted to draft as we are waiting to have UShER merged.
If that is the only thing then simply mention it in the PR description like: - [ ] Requires #13708
The dependencies and extensions seem a bit off. I guess you used https://github.com/cov-lineages/pangolin/blob/master/environment.yml as a reference? |
@Flamefire |
|
Keep it. It is better to have the direct dependencies explicit in case the others change in a later version |
@SethosII has already addressed the issue with
So that should be ok. I looked into getting the repeating lines in the extension list a bit more neat. I replaced the URLs with the GITHUB_SOURCE as suggested. However, the SOURCE_TAR_GZ will only work in one case as all the others are named v-name-version.tar.gz. I tried to put the checksums outside the |
Additionally, to move forward, added Kent_tools so I can removed them from UShER |
Test report by @boegel |
@boegelbot please test @ generoso |
@boegel: Request for testing this PR well received on generoso PR test command '
Test results coming soon (I hope)... - notification for comment with ID 907594993 processed Message to humans: this is just bookkeeping information for me, |
Test report by @boegelbot |
Test report by @boegel |
@@ -0,0 +1,80 @@ | |||
# Contribution from the NIHR Biomedical Research Centre | |||
# Guy's and St Thomas' NHS Foundation Trust and King's College London | |||
# Based on a EC provided by Paul Jähn |
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.
Just a nitpick: my last name has an e
at the end ;)
# Based on a EC provided by Paul Jähn | |
# Based on a EC provided by Paul Jähne |
'dirs': ['lib/python%(pyshortver)s/site-packages'], | ||
} | ||
|
||
sanity_check_commands = [ |
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 would also add pangolin -v
:
sanity_check_commands = [ | |
sanity_check_commands = [ | |
'pangolin -v', |
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.
lgtm
Going in, thanks @sassy-crick! |
(created using
eb --new-pr
)edit: requires
#13708(UShER),#13651(gofasta)