kpatch-build: support adding new files in patch#510
Conversation
Geting the changed objects from the patched dir, in order to support adding new files in patch. Signed-off-by: Li Bin <huawei.libin@huawei.com>
|
@libin2015 FYI, next time there's no need to open a new pull request. You can just update the old pull request by rebasing and force pushing the existing branch. |
|
👍 |
There was a problem hiding this comment.
do we need to set CHANGED=1 here? it seems that if the only thing that changed was the new file, it will bail out a few lines down with "no functional changes found".
There was a problem hiding this comment.
I think the "no functional changes found" error would still be appropriate. If they're not changing any functions, why use kpatch?
There was a problem hiding this comment.
yeah, after i thought about it for a little while, i tired to think of a situation where someone would do this i.e. call new functions in a new object without changing an existing object.
only situation i could come up with is registering in a new function in a notification chain with a kpatch load hook or something.
i'm fine with it returning an error though.
There was a problem hiding this comment.
only situation i could come up with is registering in a new function in a notification chain with a kpatch load hook or something.
Good point. I could go either way... the current patch is fine.
kpatch-build: support adding new files in patch
But if donging in this way, the previous comments based on the previous patch will be viewed odd? |
In the past we have noticed patch comments that disappear after rebasing the branch. But that problem only happened when adding a comment to a "Commit" rather than the "Files Changed" tab. I don't know if GitHub still has that quirk. But our policy to avoid this problem is to always add review comments to the "Files Changed" tab rather than the "Commits" tab. |
Geting the changed objects from the patched dir, in order to support
adding new files in patch.
Signed-off-by: Li Bin huawei.libin@huawei.com