Bring the 4th argument back.

WenyuanShao commented 6 years ago

Summary of this Pull Request (PR)

This PR trying to bring the 4th argument back. I have done this PR according to one of @phanikishoreg 's commits the hash number is e09e0ce

Intent for your PR

Choose one (Mandatory):

[ ] This PR is for a code-review and is intended to get feedback, but not to be pulled yet.
[x] This PR is mature, and ready to be integrated into the repo.

Reviewers (Mandatory):

(Specify @<github.com username(s)> of the reviewers. Ex: @user1, @user2) @gparmer , @phanikishoreg , @hungry-foolish

Code Quality

As part of this pull request, I've considered the following:

Style:

[x] Comments adhere to the Style Guide (SG)
[x] Spacing adhere's to the SG
[x] Naming adhere's to the SG
[x] All other aspects of the SG are adhered to, or exceptions are justified in this pull request
[ ] I have run the auto formatter on my code before submitting this PR (see doc/auto_formatter.md for instructions)

Code Craftsmanship:

[x] I've made an attempt to remove all redundant code
[x] I've considered ways in which my changes might impact existing code, and cleaned it up
[x] I've formatted the code in an effort to make it easier to read (proper error handling, function use, etc...)
[x] I've commented appropriately where code is tricky
[x] I agree that there is no "throw-away" code, and that code in this PR is of high quality

Testing

I've tested the code using the following test programs (provide list here): I have run llbooter_pong.sh, llbooter_test.sh, unit_schedcomp.sh, unit_schedtest.sh, microbooter.sh, unit_fprr.sh.

phanikishoreg commented 6 years ago

This is completely unrelated to this PR:

We should have the standard-basic tests as check-boxes perhaps. We should also add description around the other tests that we do, what it tests and how we think a PR is covered in that test. For ex in this case,

unit_schedtest.sh: Tests the scheduling library functionality. It is run as part of sanity and has nothing to do with the PR functionality.
unit_schedcomp.sh: Tests the scheduling library, scheduling interface functionality and also the capmgr. It covers the invocation path changed here but really doesn't completely test because the interfaces in this test do not use the 4th argument.

This would be too much to write in every PR, so I think we should just have a README per unit-test and add links here to say we ran those tests and just describe how we think our PR is tested with that test.

To accommodate that, we should change this line in the template: I've tested the code using the following test programs (provide list here): to something like: I've tested the code using the following test programs (provide a bulletted-list here with a brief description of what part of the PR is covered in that test):

OR a link to a README for Testing heading that describes how to present what you've tested.

READER: Just my random thoughts. Feel free to ignore. :-P

gparmer commented 6 years ago

@phanikishoreg I'm open to a PR to update the https://github.com/gwsystems/composite/blob/ppos/.github/PULL_REQUEST_TEMPLATE.md file. I think this is a great idea. Anyone want to do the PR?

gwsystems / composite