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

seqan 2.4.0(new formula) #549

Closed
wants to merge 1 commit into from
Closed

seqan 2.4.0(new formula) #549

wants to merge 1 commit into from

Conversation

rrahn
Copy link
Contributor

@rrahn rrahn commented Feb 7, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source FORMULA, where FORMULA is the name of the formula you're submitting?
  • Does your build pass brew audit --strict FORMULA (after doing brew install FORMULA)?

Note

Currently brew audit fails: the related issue is documented here: Homebrew/brew#5561; all suggested solutions did not work for me, but I copied the formula from the science repo and made small adaptions. Any suggestions on getting audit to work is very appreciated.


Formula/seqan.rb Outdated
desc "C++ library of sequence algorithms and data structures"
homepage "https://www.seqan.de/"
# doi "10.1186/1471-2105-9-11"
# tag "bioinformatics"
Copy link
Member

Choose a reason for hiding this comment

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

Please delete this line.

Formula/seqan.rb Outdated
# doi "10.1186/1471-2105-9-11"
# tag "bioinformatics"
# doi "https://doi.org/10.1016/j.jbiotec.2017.07.017"
# tag "Journal of Biotechnology"
Copy link
Member

Choose a reason for hiding this comment

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

Please delete this line.

Formula/seqan.rb Outdated
class Seqan < Formula
desc "C++ library of sequence algorithms and data structures"
homepage "https://www.seqan.de/"
# doi "10.1186/1471-2105-9-11"
Copy link
Member

Choose a reason for hiding this comment

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

Please use brew tap brewsci/base and then…

$ brew cite --ruby 10.1186/1471-2105-9-11
# cite D_ring_2008: "https://doi.org/10.1186/1471-2105-9-11"
Suggested change
# doi "10.1186/1471-2105-9-11"
# cite D_ring_2008: "https://doi.org/10.1186/1471-2105-9-11"

Formula/seqan.rb Outdated
homepage "https://www.seqan.de/"
# doi "10.1186/1471-2105-9-11"
# tag "bioinformatics"
# doi "https://doi.org/10.1016/j.jbiotec.2017.07.017"
Copy link
Member

Choose a reason for hiding this comment

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

Same as above use brew cite --ruby 10.1016/j.jbiotec.2017.07.017

Copy link
Member

Choose a reason for hiding this comment

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

Please move these # cite comments below head.

Formula/seqan.rb Outdated
# tag "bioinformatics"
# doi "https://doi.org/10.1016/j.jbiotec.2017.07.017"
# tag "Journal of Biotechnology"

Copy link
Member

Choose a reason for hiding this comment

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

Please delete this blank line.

Formula/seqan.rb Outdated
head "https://github.com/seqan/seqan.git"

# seqan-library installs only header files.
bottle :unneeded
Copy link
Member

Choose a reason for hiding this comment

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

Please remove these three lines 12-14.

Formula/seqan.rb Outdated

test do
# seqan-library installs only header files, so we test if version.h exists and has the right version number.
system "test -e filename", include/"seqan/version.h"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
system "test -e filename", include/"seqan/version.h"
assert_predicate include/"seqan/version.h", :exist?

Copy link
Member

@sjackman sjackman left a comment

Choose a reason for hiding this comment

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

Thank you very much for this work, @rrahn!

Formula/seqan.rb Outdated
test do
# seqan-library installs only header files, so we test if version.h exists and has the right version number.
system "test -e filename", include/"seqan/version.h"
system "grep \"SEQAN_VERSION_MAJOR 2\"", include/"seqan/version.h"
Copy link
Member

Choose a reason for hiding this comment

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

Please do not include the version number in the test.

Suggested change
system "grep \"SEQAN_VERSION_MAJOR 2\"", include/"seqan/version.h"
assert_match "SEQAN_VERSION_MAJOR", include/"seqan/version.h"

Formula/seqan.rb Outdated
system "test -e filename", include/"seqan/version.h"
system "grep \"SEQAN_VERSION_MAJOR 2\"", include/"seqan/version.h"
system "grep \"SEQAN_VERSION_MINOR 4\"", include/"seqan/version.h"
system "grep \"SEQAN_VERSION_PATCH 0\"", include/"seqan/version.h"
Copy link
Member

Choose a reason for hiding this comment

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

Please delete these two lines 32 and 33.

@sjackman sjackman self-assigned this Feb 7, 2019
@sjackman
Copy link
Member

sjackman commented Feb 7, 2019

Currently brew audit fails: the related issue is documented here: Homebrew/brew#5561; all suggested solutions did not work for me, but I copied the formula from the science repo and made small adaptions. Any suggestions on getting audit to work is very appreciated.

Try…

rm -rf /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby
rm -rf /usr/local/Homebrew/Library/Homebrew/vendor/bundle
brew style hello

@rrahn
Copy link
Contributor Author

rrahn commented Feb 7, 2019

@sjackman I did a complete fresh install of homebrew and I could get audit to work this way. I also adapted the test as you suggested, though I needed to read in the version file, otherwise the assertion failed as the pattern was matched against the path name and not the file content. Please let me know, if this needs to be revised some how. I am very new to ruby 😅. And many thanks for the quick response and review. 👍

@sjackman sjackman closed this in 34e5327 Feb 8, 2019
@sjackman
Copy link
Member

sjackman commented Feb 8, 2019

Merged. Thank you for contribution to Brewsci/bio, @rrahn ! 🏆

@rrahn
Copy link
Contributor Author

rrahn commented Feb 8, 2019

@sjackman you are welcome and many thanks for the quick support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants