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

Explicitly set tmp dir for fasterq-dump #5863

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Mar 13, 2024

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

@@ -2,6 +2,7 @@
<description>format from NCBI SRA</description>
<macros>
<import>macros.xml</import>
<token name="VERSION_SUFFIX">1</token>
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this "overwrite" the token in macros.xml?

Is this a good idea?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. why should we bump every tool if only one has changed ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the argument, but IMO this is easily missed when updating the macros.

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't that handled by the bot ? What's the process to just bump one tool ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sometimes people bump manually :)

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, so now what ? I can drop @VERSION_SUFFIX@ and put a literal 1 if that's any better

@mvdbeek

This comment was marked as outdated.

@bgruening bgruening merged commit cb858cb into galaxyproject:main Mar 14, 2024
13 checks passed
@bernt-matthias
Copy link
Contributor

Seems that this did not work. At least the toolshed still reports this as galaxy0. Also the complete repo including all three tools has been updated, or? Was a bit surprised that fasterq-dump can't be found at all as a single tool.

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