job-list: misc cleanup #5144
Merged
job-list: misc cleanup #5144
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
re-pushed with an extra commit from something I just noticed in the JobList class. I cannot find evidence that we've ever accepted "all" as a filter name. And "all" in the code only sets pending and running, not pending, running, and inactive. So it doesn't make much sense. I suspect its some left over code from development of the class? Perhaps there is something I just can't remember or some use case I can't find. But there's a cleanup commit in here for the time being. |
garlick
approved these changes
May 5, 2023
grondo
approved these changes
May 5, 2023
|
@chu11 I'm going to set MWP here in a bit unless I hear otherwise, we're trying to see if this mergify issue is affecting all PRs our just some... |
|
doh, looks like it hit us again |
|
Ok, we might have to ask the folks at mergify :-( |
✅ Branch has been successfully rebased |
Problem: The function list_inactive_cb() was removed a long time ago but the prototype for it is still in list.h. Remove the function prototype.
Problem: There is a comment describing job list filtering with the 'since' filtering. The comment is a little confusing because it does not match the code, doesn't describe how a list is sorted, and happens to use the word "since" in the description. Clarify the comment with more details.
Problem: Some tabbing was incorrect. Make tabbing good!
Problem: The wording in the --since option in flux-jobs(1) is a bit misleading, suggesting that the --since option may apply only to completed/inactive jobs. Solution: Note that the --since option applies to jobs that were active after the given timestamp, which makes it clear that --since also will list pending and running jobs.
Problem: In a test in t2800-jobs-cmd.t, both the --filter and --all option are specified, leading to unexpected output. The test happens to pass, but may not behave as intended. Remove the -a option in the test.
Problem: A comment in t2260-job-list.t is out of date. flux_job_list() has been deprecated and is no longer used. Update the comment.
Problem: The t2250-job-archive.t test does not test that active jobs are not incorrectly archived in the archive database. Add an additional test to double check for this.
Problem: The JobList class's add_filter() function checks for the special filter name "all" and sets the filter to pending and running. As far as I can tell, the special filter name "all" has never been supported or documented. In addition, it doesn't make much sense since it should set pending, running, and inactive to get all jobs. My suspicion is this may be left over code from development that did not get removed. Remove check for the filter name "all".
✅ Branch has been successfully rebased |
Codecov Report
@@ Coverage Diff @@
## master #5144 +/- ##
==========================================
- Coverage 83.13% 83.12% -0.01%
==========================================
Files 453 453
Lines 77788 77784 -4
==========================================
- Hits 64666 64659 -7
- Misses 13122 13125 +3
|
|
This is approved with MWP so I'll merge since mergify still now working. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is some misc cleanup / doc updates peeled off from another branch. It also include cleanup in
flux-job, testsuite, andflux-jobs(1).