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.

Sign up for GitHub

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

Jump to bottom

Changed stdout output in three scripts to stderr #626

Merged
merged 3 commits into from
Nov 14, 2014
Merged

Changed stdout output in three scripts to stderr #626

merged 3 commits into from
Nov 14, 2014

Conversation

b-wyss
Copy link
Contributor

@b-wyss b-wyss commented Oct 3, 2014

These need to be changed:

  • abundance-dist
  • abundance-dist-single
  • annotate-partitions
  • count-median
  • count-overlap
  • do-partition
  • extract-paired-reads
  • extract-partitions
  • filter-abund
  • filter-abund-single
  • filter-stoptags
  • find-knots
  • load-graph
  • load-into-counting
  • make-initial-stoptags
  • merge-partitions
  • normalize-by-median
  • partition-graph
  • sample-reads-randomly

These don't need to be changed:
split-paired-reads
extract-long-sequences
fastq-to-fasta
interleave-reads

@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 Oct 3, 2014

add to testlist

@ged-jenkins
Copy link

Test FAILed.

@ged-jenkins
Copy link

Test PASSed.

3 similar comments
@ged-jenkins
Copy link

Test PASSed.

@ged-jenkins
Copy link

Test PASSed.

@ged-jenkins
Copy link

Test PASSed.

@b-wyss
Copy link
Contributor Author

b-wyss commented Oct 7, 2014

@brtaylor92, @bocajnotnef, @wrightmhw mind taking a quick look at this?

@mr-c
Copy link
Contributor

mr-c commented Oct 8, 2014

And by a quick look @b-wyss means a code review :-) Where is the checklist? http://khmer.readthedocs.org/en/latest/dev/coding-guidelines-and-review.html

@b-wyss
Copy link
Contributor Author

b-wyss commented Oct 9, 2014

sorry, forgot that bit.

  • Is it mergable?
  • Did it pass the tests?
  • If it introduces new functionality in scripts/ is it tested?
    Check for code coverage.
  • 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.
  • Is it documented in the ChangeLog?
  • Was a spellchecker run on the source code and documentation after
    changes were made?

@brtaylor92
Copy link
Contributor

Needs ChangeLog; otherwise lgtm

@ged-jenkins
Copy link

Test FAILed.

@mr-c
Copy link
Contributor

mr-c commented Oct 10, 2014

retest this please

@mr-c
Copy link
Contributor

mr-c commented Oct 10, 2014

please test this

@mr-c
Copy link
Contributor

mr-c commented Oct 13, 2014

retest this please

@ged-jenkins
Copy link

Test PASSed.

2 similar comments
@ged-jenkins
Copy link

Test PASSed.

@ged-jenkins
Copy link

Test PASSed.

@SensibleSalmon
Copy link
Contributor

All looks good!

@mr-c
Copy link
Contributor

mr-c commented Oct 14, 2014

Please squash down to one commit. Thanks!

@ged-jenkins
Copy link

Test PASSed.

@mr-c
Copy link
Contributor

mr-c commented Nov 7, 2014

@b-wyss status?

@b-wyss
Copy link
Contributor Author

b-wyss commented Nov 7, 2014

Fixed, passed tests, squashed, ready to merge since last week

@mr-c
Copy link
Contributor

mr-c commented Nov 7, 2014

I still see 9 commits, not one

On Fri, Nov 7, 2014, 17:39 Brian Wyss notifications@github.com wrote:

Fixed, passed tests, squashed, ready to merge since last week

—
Reply to this email directly or view it on GitHub
#626 (comment).

@mr-c
Copy link
Contributor

mr-c commented Nov 14, 2014

Please review make pep8 and make diff_pylint_report

@mr-c
Copy link
Contributor

mr-c commented Nov 14, 2014

retest this please

mr-c added a commit that referenced this pull request Nov 14, 2014
@mr-c
Merge pull request #626 from b-wyss/fix/scriptstderr
482da12 
Changed stdout output in three scripts to stderr
@mr-c mr-c merged commit 482da12 into dib-lab:master Nov 14, 2014
@b-wyss b-wyss deleted the fix/scriptstderr branch November 14, 2014 19:09
This was referenced Dec 15, 2014
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@b-wyss @ged-jenkins @mr-c @brtaylor92 @SensibleSalmon