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

Fixed check_file_status error #860

Merged
merged 8 commits into from
Apr 1, 2015
Merged

Fixed check_file_status error #860

merged 8 commits into from
Apr 1, 2015

Conversation

TheOneHyer
Copy link
Contributor

check_file_status requires two arguments: file_path and force. find-knots.py only gave the first argument, this update adds in the force argument.

check_fiel_status requires two arguments: file_path and force. find-knots.py only gave the first argument, this update adds in the force argument.
@ged-jenkins
Copy link

Can one of the admins verify this patch?

1 similar comment
@ged-jenkins
Copy link

Can one of the admins verify this patch?

@mr-c
Copy link
Contributor

mr-c commented Mar 8, 2015

Jenkins, okay to test

@mr-c
Copy link
Contributor

mr-c commented Mar 8, 2015

Jenkins, test this please

@mr-c
Copy link
Contributor

mr-c commented Mar 30, 2015

@ged-jenkins
Copy link

Can one of the admins verify this patch?

1 similar comment
@ged-jenkins
Copy link

Can one of the admins verify this patch?

@mr-c
Copy link
Contributor

mr-c commented Mar 30, 2015

This was missed due to #358

TheOneHyer added a commit to TheOneHyer/khmer that referenced this pull request Mar 30, 2015
Change log entry for "Fixed check_file_status error dib-lab#860"
@TheOneHyer
Copy link
Contributor Author

  • [Yes ] Is it mergeable?
  • [*See note below ] Did it pass the tests?
  • [ No new functionality] If it introduces new functionality in scripts/ is it tested?
    Check for code coverage with make clean diff-cover
  • [ No changes made that would change formatting] Is it well formatted? Look at make pep8, make diff_pylint_report,
    make cppcheck, and make doc output. Use make format and manual
    fixing as needed.
  • [ No change to command -line interface] Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • [See Added change log entry for #860 #891] Is it documented in the ChangeLog?
    http://en.wikipedia.org/wiki/Changelog#Format
  • [ Yes] Was a spellchecker run on the source code and documentation after
    changes were made?

*Note: The proposed change was made on the lab server at the lab where I work, I do not have write permission to program files on the server yet (I am undergoing sys admin training and will have permissions soon). My personal machine is not powerful enough to run khmer on metagenomic data in a time effective manner. Thus I have not personally tested the change on any real data (I did email the current sys admin the fix but he did not want to implement because mr-c gave ged-jenkins permission to test and he figured you guys would do a better job testing the script).

@mr-c
Copy link
Contributor

mr-c commented Mar 30, 2015

@TheOneHyer Thanks for completing the checklist. To add the ChangeLog you can make additional commits to the branch this pull request is from: the 'master' branch of your fork of the repository.

@mr-c
Copy link
Contributor

mr-c commented Mar 30, 2015

Jenkins, ok to test

Change log entry for "Fixed check_file_status error dib-lab#860"
@TheOneHyer
Copy link
Contributor Author

I've moved my ChangeLog to this pull and closed (#891). I apologize about my sloppiness, I'm quite new to GitHub.

@mr-c
Copy link
Contributor

mr-c commented Mar 30, 2015

@TheOneHyer It is no problem! Have you seen http://khmer.readthedocs.org/en/latest/dev/getting-started.html ?

@mr-c
Copy link
Contributor

mr-c commented Mar 30, 2015

Can you make your branch mergable? We've updated the master branch since you started.

If you've been following http://khmer.readthedocs.org/en/latest/dev/getting-started.html then the following commands should work:

git fetch --all
git merge ged/master

You may need to manually fix a few conflicts using git mergetool

ChangeLog updated to match find-knots.py fix
@TheOneHyer
Copy link
Contributor Author

ready for review!

Deleted extra entry in ChangeLog
Fixed Bad Line
@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2015

Jenkins, test this please

@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2015

I tested this by executing the doc/run-corn-50m.sh script which includes a call to find-knots.py. Despite not being tested automatically by Jenkins this script successfully completed.

@@ -55,6 +55,11 @@
* tests/test_{counting_single,subset_graph}.py: make explicit the use of
floor division behavior.

2015-03-07 Alex Hyer <theonehyer@gmail.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since your pull request will be the latest addition to the project when we merge it should be at the top of the file with a date newer than the preceding entry.

@mr-c
Copy link
Contributor

mr-c commented Mar 31, 2015

@TheOneHyer Once you get your ChangeLog entry moved to the top of the file I'll merge this. Thank you for catching and fixing this bug!

@TheOneHyer
Copy link
Contributor Author

ready for review!

@mr-c mr-c merged commit e685479 into dib-lab:master Apr 1, 2015
@mr-c
Copy link
Contributor

mr-c commented Apr 1, 2015

@TheOneHyer, Congratulations on your first commit to the khmer project! Your name will be included in the release notes for the next version and you'll be listed amongst our other contributors in the next software release paper.

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