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

[INTERNAL, FEATURE] Clean up of folder and add openMS support #3

Merged
merged 8 commits into from
Sep 18, 2018

Conversation

smehringer
Copy link
Contributor

@smehringer smehringer commented Sep 17, 2018

This PR removes the unnecessary old scripts and logfiles and introduces parsing of OpenMS logs.

@jpfeuffer Please add an ssh call for the openMS logs in Jenkins. I can then update the Jenkins script to also create the openMS report.

@jpfeuffer
Copy link
Contributor

Yes I added a command. The logs go to log_files/openms.
By the way, is the process_seqan_logfiles specific to seqan? Can we rename it to process_logfiles and use it twice. Maybe with an option to specify an output file?

@smehringer
Copy link
Contributor Author

!rebuild

@smehringer
Copy link
Contributor Author

@jpfeuffer looks like it works now. The html page just has seqan statistics as a title...

@jpfeuffer
Copy link
Contributor

@smehringer Yep looks good. There is another occurrence of SeqAn for that OpenMS specific column "who". Should probably be changed, too.

@jpfeuffer
Copy link
Contributor

Ah also the plot titles sometimes.

@@ -71,6 +71,11 @@ if(!require(RColorBrewer)) {
library(RColorBrewer, lib.loc=local.lib)
}

if(!require(yaml)) {
install.packages("yaml", repos = c('http://rforge.net', 'http://cran.rstudio.org'), type = 'source', lib=local.lib)
library(RColorBrewer, lib.loc=local.lib)
Copy link

Choose a reason for hiding this comment

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

should say library(yaml, lib.loc=local.lib) ?

Copy link
Contributor Author

@smehringer smehringer Sep 18, 2018

Choose a reason for hiding this comment

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

good catch thanks. Copy and Paste errors are my specialty 🌟

@jpfeuffer
Copy link
Contributor

I think you don't need the script.basename for the render location because you added getWd in the beginning already.

@jpfeuffer jpfeuffer merged commit e0f5584 into OpenMS:master Sep 18, 2018
@smehringer
Copy link
Contributor Author

@jpfeuffer thanks for fixing the last things! 👍

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