Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented May 26, 2020

Closes #486

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2020

Codecov Report

Merging #1338 into master will decrease coverage by 0.06%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1338      +/-   ##
==========================================
- Coverage   91.72%   91.65%   -0.07%     
==========================================
  Files          83       83              
  Lines       12578    12595      +17     
==========================================
+ Hits        11537    11544       +7     
- Misses       1041     1051      +10     
Impacted Files Coverage Δ
reframe/frontend/cli.py 77.57% <44.44%> (-1.98%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6708be...19e84ed. Read the comment docs.

@ekouts
Copy link
Contributor

ekouts commented May 27, 2020

@jenkins-cscs retry all

Copy link
Contributor

@ekouts ekouts left a comment

Choose a reason for hiding this comment

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

I think we should add some unittests and maybe a comment on the documentation here about the different ways you can pass the options to --job-option.

@vkarak
Copy link
Contributor

vkarak commented May 27, 2020

I agree with @ekouts. Documentation of the -J option must be elaborated. I don't know what kind of unit tests we could add though.

Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Lgtm, overall. I just have a couple of minor comments.

@vkarak vkarak requested a review from ekouts May 29, 2020 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace scheduler-specific command-line options

4 participants