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

[develop]Simplify conditional statements in slurm conf template #2682

Closed
wants to merge 1 commit into from

Conversation

hanwen-pcluste
Copy link
Contributor

@hanwen-pcluste hanwen-pcluste commented Mar 6, 2024

Uri is required by Database section. Host is required by ExternalSlurmdbd. Therefore, to check the existence of Uri or Host, we just need to check the existence of Database or ExternalSlurmdbd.

This improvement simplifies the code and reduces the burden of mocking in testing.

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@hanwen-pcluste hanwen-pcluste requested review from a team as code owners March 6, 2024 14:18
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.48%. Comparing base (7d414fe) to head (fbe8c94).
Report is 32 commits behind head on develop.

❗ Current head fbe8c94 differs from pull request most recent head 9adc3b5. Consider uploading reports for the commit 9adc3b5 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2682   +/-   ##
========================================
  Coverage    76.48%   76.48%           
========================================
  Files           22       22           
  Lines         2220     2220           
========================================
  Hits          1698     1698           
  Misses         522      522           
Flag Coverage Δ
unittests 76.48% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

judysng
judysng previously approved these changes Mar 6, 2024
`Uri` is required by `Database` section. `Host` is required by `ExternalSlurmdbd`. Therefore, to check the existence of `Uri` or `Host`, we just need to check the existence of `Database` or `ExternalSlurmdbd`.

This improvement simplifies the code and reduces the burden of mocking in testing.

Signed-off-by: Hanwen <[email protected]>
@hanwen-pcluste hanwen-pcluste changed the title [develop][Dependencies] Update checksum for DCV on Ubuntu20 [develop]Simplify conditional statements in slurm conf template Apr 23, 2024
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.

3 participants